Repository navigation
Conversation
An invalid back-reference such as `s/\(a\)/\2/` is only detected once the flags have been read, so the diagnostic caret landed past the end of the command. Record where the highest `\N` appears in the replacement and report the error there.
There was a problem hiding this comment.
🟢 Approval recommended
The focused change preserves validation behavior and adds regression coverage, with no unresolved findings.
No open issues 🎉
What changed in this PR
Corrects sed’s invalid back-reference diagnostic so the caret points at the offending reference rather than past the command.
Changes:
- Records the first occurrence of the highest-numbered replacement reference.
- Uses that location for validation errors.
- Adds a regression test for caret placement.
| File | Description |
|---|---|
| tests/by-util/test_sed.rs | Tests caret placement with repeated references. |
| src/sed/compiler.rs | Captures and uses the reference’s source location. |
| src/sed/command.rs | Stores an optional location in replacement templates. |
Review effort: Balanced
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
==========================================
- Coverage 94.85% 94.84% -0.01%
==========================================
Files 14 14
Lines 7230 7236 +6
Branches 432 432
==========================================
+ Hits 6858 6863 +5
- Misses 371 372 +1
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 7.69%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | number_fix |
1.3 s | 1.2 s | +12.01% |
| ⚡ | access_log_subst |
2.2 s | 2.1 s | +3.55% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing sylvestre:sed-backref-caret (6ac57c8) with main (b1ea0dc)
An invalid back-reference such as
s/\(a\)/\2/is only detected after the flags are parsed, so theUUTILS_DIAGcaret landed past the end of the command. It now points at the\2, including when the replacement spans several lines.