Skip to content

fix(search): use xargs -0 for null-delimited filelist to handle space…#701

Closed
naman616 wants to merge 1 commit into
DeusData:mainfrom
naman616:fix-search-code-macos-arm64
Closed

fix(search): use xargs -0 for null-delimited filelist to handle space…#701
naman616 wants to merge 1 commit into
DeusData:mainfrom
naman616:fix-search-code-macos-arm64

Conversation

@naman616

Copy link
Copy Markdown

…s in path (#670)

write_scoped_filelist wrote newline-delimited absolute paths; BSD xargs split each entry on whitespace, so a space in root_path (e.g. /Users/John Smith/...) caused grep to receive broken path fragments, silently producing zero matches.

Fix: write null-delimited paths (fputc '\0') and invoke xargs -0 so each path is passed as a single argument regardless of embedded spaces or metacharacters. Also add -H to the non-scoped grep -rn fallback so BSD grep always emits the filename prefix that collect_grep_matches requires.

Regression test: search_code_space_in_path_issue670 creates a project rooted at a directory containing a space, inserts an indexed node, and asserts that search_code returns a match — reproducing the exact reported failure scenario.

What does this PR do?

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

@naman616 naman616 requested a review from DeusData as a code owner June 29, 2026 17:17
…s in path (DeusData#670)

write_scoped_filelist wrote newline-delimited absolute paths; BSD xargs split
each entry on whitespace, so a space in root_path (e.g. /Users/John Smith/...)
caused grep to receive broken path fragments, silently producing zero matches.

Fix: write null-delimited paths (fputc '\0') and invoke xargs -0 so each path
is passed as a single argument regardless of embedded spaces or metacharacters.
Also add -H to the non-scoped grep -rn fallback so BSD grep always emits the
filename prefix that collect_grep_matches requires.

Regression test: search_code_space_in_path_issue670 creates a project rooted
at a directory containing a space, inserts an indexed node, and asserts that
search_code returns a match — reproducing the exact reported failure scenario.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: naman616 <namanlodha616@gmail.com>
@naman616 naman616 force-pushed the fix-search-code-macos-arm64 branch from ef1e7a2 to 5df9173 Compare June 29, 2026 18:22
@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jun 29, 2026
@DeusData

DeusData commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Huge thanks for opening this PR and for the work you put into it.

The maintainer shop is currently full, so this may sit for a bit before it gets a proper review. We will come back to this as soon as possible with real feedback; I wanted to make sure it did not sit unacknowledged in the meantime.

@DeusData

DeusData commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Thanks, this looks focused. Before merge, please rewrite the commit metadata to remove generated/co-author/session attribution. DCO otherwise appears to be present on the non-merge commit.

@DeusData

DeusData commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Thank you for digging into #670 — your diagnosis of the newline-delimited filelist breaking on spaced paths was correct. While this was open, the same class got fixed on main via 3dd2471 (#751) plus 32bb56e (-Hn), with one crucial platform split your CI run surfaced the hard way: Windows' PowerShell Get-Content path can't consume NUL-delimited lists, which is why main keeps newline delimiting there — and why this branch's own Windows job went red. Since the Unix fix is already on main and the unconditional NUL can't ship as-is, we're closing this as superseded. Your instinct on the root cause was right, and we appreciate the regression test shape — the merged version guards the same scenario. Thanks again!

@DeusData DeusData closed this Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants