grep: spell walked paths with / on Windows when UUTILS_PATH_SEPARATOR=/ by mutewinter · Pull Request #143 · uutils/grep

mutewinter · 2026-10-15T18:17:33Z

Related: uutils/findutils#906 (same variable for find), uutils/coreutils#12742, microsoft/coreutils#107.

Problem

A recursive search on Windows prints the paths it walks with \:

PS> grep -r erosion reports
reports\b.md:Flags: erosion near outfall
PS> grep -r erosion C:\Users\me\fx\reports
C:\Users\me\fx\reports\b.md:Flags: erosion near outfall

MSYS2, Cygwin and Git Bash users, and programs that drive grep from a POSIX-style shell, expect reports/b.md and have no way to ask for it. A tool parsing the output cannot just replace \ afterwards, because the matched line may contain backslashes of its own.

Cause

Searcher::process_dir hands process_file the DirEntry::path() that walkdir built with Path::join, which uses \ on Windows; the output code prints that path as is.

Fix

A cfg(windows) forward_slashes field on Config, set when UUTILS_PATH_SEPARATOR=/, and in process_dir the walked path is respelled with / before it reaches process_file, so the prefix, -l/-L/-c names and error messages agree. It also accepts slash, because MSYS2 and Git Bash rewrite an environment variable whose value is a bare / into their install folder before a native program sees it (UUTILS_PATH_SEPARATOR=/ python -c ... in Git Bash reads C:/Program Files/Git/, checked with 2.53.0.windows.2); the users the variable is for could not otherwise set it. This mirrors ripgrep's --path-separator; the variable keeps GNU's option space untouched and is the same one proposed for uutils find.

Scope

Unchanged: the default on every platform; values other than /; non-Windows builds; file operands named on the command line, which GNU prints as given; verbatim \\?\ and device \\.\ paths.

Tests

Four tests in tests/test_grep.rs: -r output, -rl output with / and with slash, and that another value keeps std::path::MAIN_SEPARATOR. On Linux and macOS the first three assert existing behavior; the Windows CI leg exercises the change. Full CI (including windows-latest) and GnuTests pass in my fork; cargo clippy --all-targets -- -D warnings is clean for the host and x86_64-pc-windows-gnu.

Authored with Claude Opus 5.5

A recursive search on Windows joins the paths it walks with a backslash, so grep -r prints deep\sub\c.txt:... where MSYS2, Cygwin and Git Bash users, and scripts written for them, expect deep/sub/c.txt. With UUTILS_PATH_SEPARATOR=/ the walked paths use / instead. Operands named on the command line keep their spelling, as GNU grep prints them as given. Other values, and other platforms, are unchanged. It also accepts UUTILS_PATH_SEPARATOR=slash, because MSYS2 and Git Bash rewrite an environment value of a bare / into their install folder before a native program sees it.

Copilot

🟡 Changes recommended

Lossy path conversion can prevent valid Windows files from being opened or cause a different file to be searched.

1 open finding

Avoid lossy path conversion for Windows filenames with surrogates · New

What changed in this PR

Adds opt-in forward-slash paths for recursive Windows searches without changing command-line file operands.

Changes:

Recognizes UUTILS_PATH_SEPARATOR=/ and slash on Windows.
Converts walked paths while preserving verbatim and device paths.
Adds four separator behavior tests.

File	Description
tests/test_grep.rs	Tests recursive output and separator settings.
src/searcher.rs	Converts walked paths to forward slashes.
src/lib.rs	Reads the Windows separator preference.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

lhecker · 2026-10-19T16:33:39Z

+    let s = path.to_string_lossy();
+    if !s.contains('\\') || s.starts_with("\\\\?\\") || s.starts_with("\\\\.\\") {
+        return None;
+    }
+    Some(std::path::PathBuf::from(s.replace('\\', "/")))


The AI is right and wrong.

The encoding of Path is an implementation detail, but we can relatively safely assume that it'll always be some superset of ASCII (currently WTF-8), as long as we have some unit tests to cover it.

This allows you to inspect the path as an OsString and directly mutate the underlying bytes. Don't use encode_wide for something like this - that's really stupid for it to suggest.

codspeed · 2026-10-15T19:00:27Z

Merging this PR will not alter performance

⚠️

3 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

fixed_string

search_pattern

literal_no_match

✅ 10 untouched benchmarks
⏩ 17 skipped benchmarks¹

_{Comparing mutewinter:path-separator-env (2f65530) with main (01fb6c3)}

17 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

lhecker · 2026-10-19T16:28:55Z

+    /// Join the paths a recursive search walks with `/` rather than `\`, as
+    /// `UUTILS_PATH_SEPARATOR=/` asks, or `=slash`, which MSYS2 and Git Bash
+    /// pass through unchanged where they rewrite a bare `/`.


I don't understand this comment.

lhecker · 2026-10-19T16:33:39Z

+    let s = path.to_string_lossy();
+    if !s.contains('\\') || s.starts_with("\\\\?\\") || s.starts_with("\\\\.\\") {
+        return None;
+    }
+    Some(std::path::PathBuf::from(s.replace('\\', "/")))


The AI is right and wrong.

The encoding of Path is an implementation detail, but we can relatively safely assume that it'll always be some superset of ASCII (currently WTF-8), as long as we have some unit tests to cover it.

This allows you to inspect the path as an OsString and directly mutate the underlying bytes. Don't use encode_wide for something like this - that's really stupid for it to suggest.

lhecker · 2026-10-19T16:35:33Z

FWIW, I'm not happy with this PR aside from my review comments. I fail to see the point of UUTILS_PATH_SEPARATOR - both figuratively and literally. There appears to be no clear justification for its existence in the linked issues.

Copilot AI balanced review requested due to automatic review settings October 15, 2026 18:17

Copilot started reviewing on behalf of mutewinter October 15, 2026 18:31

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

lhecker requested changes Oct 19, 2026

View reviewed changes

Navigation Menu

Repository navigation

grep: spell walked paths with / on Windows when UUTILS_PATH_SEPARATOR=/ - #143

grep: spell walked paths with / on Windows when UUTILS_PATH_SEPARATOR=/#143
mutewinter wants to merge 1 commit into
uutils:mainfrom
mutewinter:path-separator-env

mutewinter commented Oct 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

lhecker Oct 19, 2026

Uh oh!

codspeed Bot commented Oct 15, 2026

Uh oh!

lhecker Oct 19, 2026

Uh oh!

lhecker Oct 19, 2026

Uh oh!

lhecker commented Oct 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mutewinter commented Oct 15, 2026

Problem

Cause

Fix

Scope

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

lhecker Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 15, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

lhecker Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

lhecker commented Oct 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants