Repository navigation
sed: don't restart a numeric range when a branch or D revisits its line - #675
sarankumar1325 wants to merge 1 commit into
Conversation
A range whose first address is a line number and that ends on that same line (2,+0, 2,2, 2,0, 2,~0, 2,1, 2,/re/ matching line 2, 2,$ on the last line) was selected again each time a backward b/t branch or a D restart re-evaluated the command on that line, so ':y;2,+0by' never terminated. A numeric start equal to the current line now also requires that the range hasn't been entered yet, and a $ end that matches where the range starts closes it there. Fixes uutils#648
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.
Merging this PR will improve performance by 2.81%1 warning
Performance Changes
Tip Curious why performance improved? Comment Comparing 
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #675 +/- ##
=======================================
Coverage 95.39% 95.39%
=======================================
Files 14 14
Lines 7665 7670 +5
Branches 457 459 +2
=======================================
+ Hits 7312 7317 +5
Misses 352 352
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:
|
Fixes #648.
In
applies(), a numeric first address equal to the current line went throughmatch_address, which is true every time the line is re-evaluated. So a range that starts and ends on the same line was selected again after each backwardb/tbranch orDrestart, and:y;2,+0bynever terminated.Changes:
!command.range_started. It's the same rule thelinenum > narm already uses, so a line-number range is entered only once per input (or per file with-s/-i, wherereset_latched_address_rangesclears it).$end that already matches on the start line (N,$when N is the last line) closes the range there instead of latching it, so revisiting the line doesn't select it a second time.Checked against GNU sed 4.9 (same output now):
:a;2,+0{s/x/y/;ta}(also2,2,2,0,2,~0,2,1,2,$)a\nxxxa\nyxxa\nyyy:y;2,+0by1\n2\n31\n2\n3:a;2,/x/{s/x/y/;ta}a\nxxx\nxxa\nyyx\nxxa\nyyy\nyxAlso unchanged against GNU:
2,3p,2,2p;2,1p,1,$p,/b/,$p,$!N;3,$P;D, and2,$pand a branching2,$loop over two files with and without-s.Tests (tests/by-util/test_sed.rs):
test_numeric_range_not_restarted_by_branch(6 scripts),test_numeric_range_regex_end_not_restarted_by_branchandtest_numeric_range_branch_terminates(5 s timeout) fail on main and pass with this change.test_numeric_range_not_restarted_by_d_restartpasses on both and guards theDpath.cargo fmt --checkandcargo clippy --all-targets -- -D warningsare clean.cargo test: everything passes excepttest_invalid_unicode_escape_does_not_panic, the intermittent broken-pipe failure that #674 fixes (it fails on main too).Not in this PR: GNU also stops re-selecting a range's last line once it has closed on it (
:a;2,3{s/x/y/;ta}givesyxon line 3 under GNU,yyhere, both before and after this change). That's the active-range path, not the restart this issue is about. I can send it separately if you want it.I used an AI assistant for this change and reviewed the code and tests myself.