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

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.

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

Merging this PR will not alter performance

1 warning

⚠️ 3 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

✅ 10 untouched benchmarks
⏩ 17 skipped benchmarks¹

_{Comparing mutewinter:path-separator-env (88b8060) with main (0be8c54)}

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.

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.

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

mutewinter · 2026-10-27T15:26:46Z

Fair, and sorry the description and comment were hard to follow. The linked issues are adjacent (separator style in ls glob output) and don't justify this on their own. The actual reason:

We run uutils grep and find from an agent's shell on Windows, where every path is spelled with /. grep -r there prints reports\b.md, and a program reading the output can't safely turn \ back into /, since the matched line may contain backslashes too. In our corpus 4 of 33 grep invocations differed from GNU only because of this. The stronger case is find (uutils/findutils#906), where -path/-prune patterns written with / never match; this PR keeps grep consistent with it.

I've rewritten the doc comment and moved the conversion onto the OsStr encoded bytes, with Windows unit tests (including an unpaired surrogate). If you'd rather not carry an env var for one downstream consumer, I'll close this.

lhecker · 2026-10-27T17:01:24Z

/ = \ is only true for Win32 paths on Windows, so your assumptions about this being safe is to do is a bit shaky. Your harness should be able to deal with \ path separators at any point in its pipeline as if it was a /. This certainly works perfectly fine with GitHub Copilot, for instance.

So, what actually breaks in your harness when you encounter a \?

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

mutewinter force-pushed the path-separator-env branch from 2f65530 to 88b8060 Compare October 27, 2026 15:11

Copilot AI balanced review requested due to automatic review settings October 27, 2026 15:11

Copilot AI reviewed Oct 27, 2026

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 •

edited

Loading

Uh oh!

codspeed Bot commented Oct 15, 2026 •

edited

Loading

Uh oh!

lhecker Oct 19, 2026

Uh oh!

lhecker Oct 19, 2026 •

edited

Loading

Uh oh!

lhecker commented Oct 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mutewinter commented Oct 27, 2026

Uh oh!

lhecker commented Oct 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhecker commented Oct 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mutewinter commented Oct 27, 2026

Uh oh!

lhecker commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lhecker Oct 19, 2026 •

edited

Loading

codspeed Bot commented Oct 15, 2026 •

edited

Loading

lhecker Oct 19, 2026 •

edited

Loading

lhecker commented Oct 27, 2026 •

edited

Loading