Repository navigation
sed: point the invalid back-reference caret at the reference - #609
Conversation
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
=======================================
Coverage 95.10% 95.10%
=======================================
Files 14 14
Lines 7490 7499 +9
Branches 443 444 +1
=======================================
+ Hits 7123 7132 +9
Misses 366 366
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 3.52%1 warning
Performance Changes
Tip Curious why performance improved? Comment Comparing |
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.
compile_replacement records a location for every \N it reads, so the location is only absent when no group is referenced and the check cannot fire. Bind it in the condition instead of keeping an untested fallback, and cover a replacement continued on a second line.
6ac57c8 to
9a6bd7d
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
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.
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.