Skip to content

Fix ripgrep compatibility issues #194–#201 - #202

Merged
Shengyu Fu (shengyfu) merged 11 commits into
mainfrom
shengyfu-ripgrep-compatibility-fixes
Oct 13, 2026
Merged

Shengyu Fu (shengyfu) merged 11 commits into
mainfrom
shengyfu-ripgrep-compatibility-fixes

Conversation

@shengyfu

@shengyfu Shengyu Fu (shengyfu) commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Fixes the eight open ripgrep-compatibility issues. Each reported case now matches ripgrep 15.2.0 with --no-index, an on-disk index, tgrep serve, and shared daemons v1 and v2.

Fixes

Issue Change
#194 -e/-f patterns accumulate in command-line order instead of being replaced by those given after search.
#195 Globs take precedence over types, as in ripgrep. A matching glob decides alone, and when any positive glob is given, files matching none are excluded.
#196 -t/-T apply in argument order, so the last selection matching a file wins (-T rust -t rust searches Rust files).
#197 -i, -s and -S override one another; the last one wins.
#198 -U -r rewrites each block of lines joined by matches as a whole, following ripgrep's multiline printer for -o, context, --vimgrep, -b, --column, -m and --passthru. -o -r no longer prints nothing, and the server exits 0 for it. With context, a file whose rewrites print nothing adds a -- only where ripgrep would search one file at a time (--sort, -j1, a single CPU), as its parallel search skips empty output.
#199 --json -r keeps the source lines and submatch text and offsets, and adds submatches[].replacement.
#200 -f/--file is repeatable.
#201 --type-add/--type-clear apply in command-line order; --type-list shows the result. Definitions are checked before any backend is asked, so an invalid one fails the same way with or without a server.

These order-sensitive flags (-e, -f, -t, -T, --type-add, --type-clear, -i, -s, -S) keep their command-line order on both sides of any subcommand, nested ones included. For example, tgrep --type-clear rust index --type-add 'rust:*.txt' --type-list lists rust: *.txt, like rg --type-clear rust --type-add 'rust:*.txt' --type-list.

Also fixed

Zero-length matches broke the line prescan added in #102. Patterns like \b and \< match empty at a word's edge but not in an empty string, so they reach the prescan. tgrep --no-index '\b' <file> panicked in debug builds and printed every line twice in release builds, and \< skipped lines that start with a word. Each match now covers the lines from its start through its end. A match ending at a line's start covers that line too, since find_iter may have skipped an empty match there (the \< in \s|\<). With scan, local index and server, \b, \<, \>, \b{start}, \b{end} and alternations like \s|\< select the same lines as ripgrep.

Mixed old and new clients and servers

Each new behavior is opt-in on the wire and acknowledged in the result:

  • Clients send type_ops, ordered_filters, multiline_replace or report_replacements only when the result depends on them. Type definitions without a -t/-T are not sent, and -m0, which prints no lines, sends neither replacement field. ordered_filters asks for ripgrep's filter order: -t/-T in argument order, and a positive glob overriding types.
  • Servers and shared daemons (v1 and v2) accept these fields. They reply with the acks ordered_filters, multiline_replace and report_replacements.
  • A new client rejects a reply that lacks an ack it needs. It falls back with a diagnostic: to the local index for tgrep serve, or to a scan for shared views. It never prints results computed the old way.
  • An old client never sends the fields, so a new server keeps the old behavior for that client: the old rendering, and a file must pass both the globs and the types.

Tests

  • Unit tests cover block rendering, separators, JSON replacements, and flag order across top-level and nested subcommands.
  • ripgrep_compat.rs checks each reported case with scan, local index and server, including the -- around hidden rewrites in both of ripgrep's search modes, and that -U -r finds no match in an empty file, with or without context.
  • Unit tests and ripgrep_compat.rs cover zero-length matches with scan, local index and server: \b at offset 0, \< at a later line's start, and the \< hidden in \s|\<.
  • A fake old server checks that a reply missing an ack is rejected and the client falls back.
  • shared_daemon.rs covers v1 and v2 shared views.
  • tgrep serve and shared daemons v1 and v2 are tested to let a positive glob override types only for requests that ask for it.
  • Run by hand, the pinned legacy binary searching a new tgrep serve keeps its old -t/-T/-g results.
  • Locally on Windows: cargo fmt --check, clippy with managed-test-hooks, the Rust 1.88.0 check, and the tgrep-cli suite with managed-test-hooks and the pinned legacy binaries all pass, with two test threads for shared_daemon as CONTRIBUTING.md prescribes.

Known differences (unchanged)

  • --json -U emits one match event per covered line, while ripgrep emits one per block. With -r, the first fragment carries the whole replacement and the rest carry "".
  • Without -r, multiline continuation lines show columns relative to the line, while ripgrep repeats the starting column.
  • An empty --files listing still exits 0 (ripgrep exits 1).
  • In a file without a final newline, -o, --column, --count-matches, --vimgrep and -r report an empty match at the very end, which ripgrep drops: for x\nbbb, rg -o '\b' reports three matches and tgrep four.

The README documents the new ordering rules, the JSON replacement field, and the multiline -r behavior.

Fixes #194, #195, #196, #197, #198, #199, #200, #201

Shengyu Fu (shengyfu) and others added 2 commits October 12, 2026 10:10
…lags

Keep global -e/-f patterns given before the search subcommand (#194), let
a positive glob override a conflicting type filter like ripgrep (#195),
apply -t/-T in command-line order (#196) along with --type-add and
--type-clear (#201), make the last of -i/-s/-S win (#197), and accept
repeated -f/--file inputs alongside -e in order (#200). Ordered type
operations reach the server and shared daemon through a type_ops
parameter; clients require an ordered_filters acknowledgement before
trusting a reply whose result depends on that order.

Co-authored-by: Copilot App <[email protected]>
`-U --replace` now rewrites each block of lines joined by matches as a
whole, following ripgrep's multiline printer: the rewritten text is printed
line by line with line numbers counting up from the block's first line, and
context gaps are measured from the source lines. `-o -r` prints each
rewritten match instead of nothing.

`--json --replace` keeps the source `lines`, submatch text and offsets, and
adds `submatches[].replacement`, as ripgrep does.

Clients send `multiline_replace` and `report_replacements` only when the
result depends on them. `tgrep serve` and shared daemons v1 and v2
acknowledge both; a reply without the needed ack makes the client fall back
with a diagnostic instead of printing the old rendering.

Tests cover each reported case on scan, local index, server and shared
views, plus a fake old server. The README documents the ordering rules,
the JSON `replacement` field and multiline `-r` behavior. Also resolves
clippy lints.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Capability requests currently force fallbacks for searches that do not depend on the new rendering.

1 open finding
What changed in this PR

Aligns CLI parsing, filtering, replacement rendering, and server protocols with ripgrep 15.2.0.

Changes:

  • Preserves argument ordering for patterns, case flags, and type operations.
  • Implements glob precedence and multiline/JSON replacement parity.
  • Adds protocol capability acknowledgements and cross-backend tests.
File Description
README.md Documents compatibility behavior.
tgrep-core/​src/​filetypes.rs Adds ordered type operations.
tgrep-cli/​src/​main.rs Reconstructs ordered CLI arguments.
tgrep-cli/​src/​glob_filter.rs Adds glob precedence decisions.
tgrep-cli/​src/​matching.rs Implements replacement blocks and metadata.
tgrep-cli/​src/​output.rs Renders blocks and JSON replacements.
tgrep-cli/​src/​search.rs Integrates filtering, requests, and acknowledgements.
tgrep-cli/​src/​serve.rs Supports new search protocol fields.
tgrep-cli/​src/​serve/​shared/​server.rs Adds shared-v1 capabilities.
tgrep-cli/​src/​serve/​shared/​managed.rs Adds shared-v2 capabilities.
tgrep-cli/​tests/​ripgrep_compat.rs Covers reported compatibility cases.
tgrep-cli/​tests/​shared_daemon.rs Tests shared-v1 behavior.
tgrep-cli/​tests/​shared_daemon/​managed.rs Tests shared-v2 behavior.

🧠 Review effort: Balanced


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

Comment thread tgrep-cli/src/search.rs Resolved
`-U --replace` now asks for `multiline_replace` only when the pattern can
match a line terminator. Any other pattern prints line by line, which an
older server already does. `--json --replace` no longer asks for
`report_replacements` with `-v` or `-q`, which print no submatches.

The acknowledgement check now reads the request, so it can never require
a field the client did not send. An older server's reply is then used
rather than refused for searches it renders identically.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

New servers apply glob-over-type precedence to legacy clients despite the stated opt-in mixed-version contract.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread tgrep-cli/src/serve.rs Resolved
Comment thread tgrep-cli/src/serve/shared/server.rs Resolved
A server applied ripgrep's filter order to every request, so a client
that predates it, sending only `glob` and `types`, saw a positive glob
reinclude files its `-t`/`-T` had excluded. The client now sends
`ordered_filters` when its result depends on that order, and the TCP
server and shared daemons keep the old type and glob intersection for
requests without it. The client refuses a reply that does not
acknowledge the request, as before.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 01:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Empty rewritten files can emit stray separators, and definition-only type operations unnecessarily force legacy-server fallbacks.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread tgrep-cli/src/output.rs Resolved
Comment thread tgrep-cli/src/search.rs Resolved
Shengyu Fu (shengyfu) and others added 2 commits October 12, 2026 19:22
ripgrep writes a file separator, and a context break before a block that
does not follow on from the last one, before it knows whether a multiline
rewrite leaves any row to print. A block the rewrite hides therefore keeps
its `--` (and its path with --heading), even at the end of the output.
tgrep already matches this on every backend; pin it with ripgrep 15.2.0's
output.

Co-authored-by: Copilot App <[email protected]>
Without a -t/-T, --type-add/--type-clear definitions cannot change which
files are searched, yet a definition-only invocation still sent type_ops
and ordered_filters, so an older server's reply was refused and the client
fell back. Leave the definitions, type_ops and ordered_filters out of the
request unless a selection consumes them. Validate the definitions before
asking any backend, as ripgrep does, so an invalid --type-add fails the
same way with or without a server instead of reporting a server failure.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 02:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The extensive cross-backend protocol and multiline-rendering changes warrant final human validation despite strong test coverage.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

ripgrep writes the `--` between files in one of two ways. Searching one
file at a time (under --sort, -j1 or on a single CPU), its printer writes
it as soon as a file reports a line, even if the rewrite then hides it.
Searching in parallel, it buffers each file's output and writes the
separator only before a buffer that holds something, so a file whose
-U -r rewrites print nothing adds no `--`. tgrep always did the former.

Hold a file boundary's `--` until the next file prints something, unless
ripgrep would search sequentially.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 03:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Empty-file multiline matches can disappear with context, while -m0 replacement queries unnecessarily trigger compatibility fallbacks.

1 open finding
Previously missed (2)

In code that hasn't changed since last review

Medium severity Avoid requesting capabilities when max_count is zero

tgrep-cli/​src/​search.rs:337

-m0 guarantees that FileMatches::find emits no rows, but this still causes needs_replace_blocks to send the new capability. Against an older server that needlessly rejects the reply and, for a shared view, triggers a filesystem scan even though block rendering cannot affect the output. Exclude max_count == Some(0) from this predicate.

Medium severity Disable replacement reporting when max_count is zero

tgrep-cli/​src/​search.rs:355

JSON with -m0 cannot contain any submatches, yet this requests report_replacements and forces an unnecessary old-server/shared-view fallback. Gate this capability on max_count != Some(0) just like other output modes that cannot emit replacements.

🧠 Review effort: Balanced

Comment thread tgrep-cli/src/matching.rs Resolved
`-m0` prints no lines, so `-U --replace` has no block to rewrite and
`--json --replace` has no submatch to report. Requesting either capability
anyway made a search fall back from an older server or shared view that
lacked it, for output that could not differ.

Also pin that an empty file reports no match under `-U --replace`, with or
without context, as ripgrep does: there is no zero-width match to keep.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 03:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Multiline replacement reparses the regex once per matching file, creating avoidable scaling overhead.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Avoid reparsing regex HIR for every matching file

tgrep-cli/​src/​matching.rs:1071

can_match_newline reparses the compiled regex into HIR here for every matching file. Since newline capability is constant for a matcher, -U -r over many small matching files now performs the same syntax parse once per file (and in server mode after the client already performed it when building the request). Cache this property on the matcher or compute it once before entering the per-file search loop.

🧠 Review effort: Balanced

`-U --replace` only rewrites a file's matches as blocks when the pattern
can match `\n`. FileMatches::find answered that by parsing the pattern
into HIR again for every matching file, on the client and on the
server, so the cost grew with the number of `-f` patterns times the
number of matching files.

The linear-time matcher now works the answer out the first time it is
asked and keeps it, so a search parses its pattern once, whichever
thread gets there first. The `--multiline-dotall` flag the regex was
compiled with lives beside it, which retires the per-file
MatchOptions.dotall and the server's SearchOpts.dotall copies.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 04:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Ordered global arguments are still lost when --type-list is combined with a non-search subcommand.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Medium severity Preserve ordered global options across all subcommand scopes

tgrep-cli/​src/​main.rs:883

The scope-preserving reparse only runs for search, even though these options are global and --type-list is handled before subcommand dispatch. As the comment below notes, Clap otherwise keeps only the deepest scope's global values. Thus a valid command such as tgrep --type-clear rust index --type-add rust:*.txt --type-list drops the parent clear and lists both the built-in *.rs and *.txt, rather than only *.txt. Preserve ordered globals across every subcommand scope (including nested scopes), not only search.

🧠 Review effort: Balanced

Clap keeps only the deepest scope's values of a global flag, so the
reparse that rebuilds -e/-f, the type flags and the case flags in
command-line order has to cover every subcommand, not just `search`.
`--type-list` is honored under any subcommand, so
`--type-clear rust index --type-add rust:*.txt --type-list` listed an
empty rust type instead of `rust: *.txt` as rg does.

The reparse now makes those flags local at every level, nested `shared`
commands included, and reads each level of the subcommand chain in
order. It runs only when a subcommand is present and an ordered flag
was used.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 04:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes parsing, rendering, and mixed-version protocols across every backend, warranting final human validation despite strong test coverage.

0 open findings

🧠 Review effort: Balanced

The line prescan from #102 assumed every match was non-empty and took
`end - 1` as the match's last byte. Patterns like `\b` and `\<` match
empty at a word's edge without matching `""` on their own, so they
reached it. An empty match at offset 0 underflowed, which panicked in
debug builds and printed lines twice in release builds, and one at a
later line's start gave a reversed range that dropped the line.

Each match now covers the lines from its start through its end offset.
That also covers the line after a match ending at a `\n`, where
`find_iter` skips an empty match at the previous match's end, like the
`\<` before `bbb` in `x\nbbb` for `\s|\<`.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 05:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The cross-cutting parser, rendering, and mixed-version protocol changes warrant final human validation despite strong test coverage.

0 open findings

🧠 Review effort: Balanced

@shengyfu
Shengyu Fu (shengyfu) merged commit 95ea48d into main Oct 13, 2026
16 checks passed
@shengyfu
Shengyu Fu (shengyfu) deleted the shengyfu-ripgrep-compatibility-fixes branch October 13, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Global -e/-f patterns before search are silently discarded when supplied again after it

2 participants