Add support for address range commands (GNU extension) by LoukasPap · Pull Request #260 · uutils/sed

LoukasPap · 2026-01-12T15:32:52Z

Add GNU extension for 2 supported addresses for a, i and = commands
Make n_addr conditional based on POSIX flag

github-actions · 2026-01-12T15:35:47Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 8 / FAILED: 56 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 8 / FAILED: 56 / SKIPPED: 0

codecov · 2026-01-12T15:36:49Z

Codecov Report

❌ Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.09%. Comparing base (ad1f3d2) to head (941c695).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/sed/compiler.rs	90.32%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #260      +/-   ##
==========================================
+ Coverage   82.07%   82.09%   +0.02%     
==========================================
  Files          13       13              
  Lines        5445     5452       +7     
  Branches      293      297       +4     
==========================================
+ Hits         4469     4476       +7     
  Misses        974      974              
  Partials        2        2

Flag	Coverage Δ
macos_latest	`82.53% <90.32%> (+0.02%)`	⬆️
ubuntu_latest	`82.65% <90.32%> (+0.02%)`	⬆️
windows_latest	`0.00% <0.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-01-16T08:23:06Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 8 / FAILED: 56 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 8 / FAILED: 56 / SKIPPED: 0

dspinellis · 2026-01-20T14:28:13Z

Thank you! This is a GNU extension, so the feature should not be available under --posix. Please also squash all commits into one.

LoukasPap · 2026-01-20T15:51:15Z

Got it! I'll fix these and push again.

LoukasPap · 2026-01-20T17:32:25Z

Found out that the problem happens with the l (list) command too (we allow address range with --posix enabled but it should not be allowed). Should I fix it here or open a different issue for this?

dspinellis · 2026-01-20T17:56:14Z

I'd suggest one commit for all commands. Try to unify the check for POSIX.

github-actions · 2026-01-21T16:10:01Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

github-actions · 2026-01-21T16:22:38Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

github-actions · 2026-01-21T16:30:55Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

LoukasPap · 2026-01-21T17:11:30Z

Squashed all my commits having removed the merge commit which was not mine.

The POSIX check now happens in one place. I will update the README.md too.

src/sed/compiler.rs

github-actions · 2026-01-21T17:30:33Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

github-actions · 2026-01-22T17:32:19Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

dspinellis · 2026-01-22T17:37:27Z

I see you're progressing! Try running the various CI tasks locally to avoid CI failures. Please also close the PRs that you've folded into this one.

codspeed-hq · 2026-01-22T17:37:58Z

Merging this PR will not alter performance

✅ 11 untouched benchmarks

_{Comparing LoukasPap:main (941c695) with main (25efbc0)}

LoukasPap · 2026-01-22T17:57:21Z

These two tests are failing because the supported addresses are now 2 and not 1 (like the tests assert):

test_lookup_text_command

sed/src/sed/compiler.rs

Lines 1309 to 1314 in 398ee76

    
           #[test] 
        
           fn test_lookup_text_command() { 
        
               let (lines, line) = make_providers("123abc"); 
        
               let cmd = get_cmd_spec(&lines, &line, 'a').unwrap(); 
        
               assert_eq!(cmd.n_addr, 1); 
        
           }

test_valid_command_spec

sed/src/sed/compiler.rs

Lines 1416 to 1425 in 398ee76

    
           #[test] 
        
           fn test_valid_command_spec() { 
        
               let lines = ScriptLineProvider::with_active_state("input.sed", 4); 
        
               let line = char_provider_from("a"); // valid command 
        
               let result = get_verified_cmd_spec(&lines, &line, 1); 
        
               assert!(result.is_ok()); 
        
               let spec = result.unwrap(); 
        
               assert_eq!(spec.n_addr, 1); 
        
           }

Would it be "correct" to change the tests to check for 2 addresses; What I mean is, do these tests represent the standard sed behavior (1 address) and not the 'GNU-extended' behavior (with 2 addresses) and thus changing them would be wrong?

I hope I am clear :)

src/sed/command.rs

dspinellis · 2026-01-22T18:04:36Z

I would expect that rather than storing / returning a number you would store

These two tests are failing because the supported addresses are now 2 and not 1 (like the tests assert):
1. `test_lookup_text_command`
   https://github.com/uutils/sed/blob/398ee76b79ba52768c1df214d516b2ad5c173882/src/sed/compiler.rs#L1309-L1314

2. `test_valid_command_spec`
   https://github.com/uutils/sed/blob/398ee76b79ba52768c1df214d516b2ad5c173882/src/sed/compiler.rs#L1416-L1425
Would it be "correct" to change the tests to check for 2 addresses; What I mean is, do these tests represent the standard sed behavior (1 address) and not the 'GNU-extended' behavior (with 2 addresses) and thus changing them would be wrong?

I hope I am clear :)

I suggest that you also store and process n_addr_posix and add corresponding tests.

dspinellis

Please consider the initial comments I made on the code and fix the CI errors. Will then follow-up with a more complete review.

src/sed/compiler.rs

LoukasPap · 2026-01-24T12:58:15Z

I see you're progressing! Try running the various CI tasks locally to avoid CI failures. Please also close the PRs that you've folded into this one.

Thank you for helping me out! Indeed, I recently noticed I could run the CI tasks locally. It will be much faster than pushing and then checking for errors.

Please consider the initial comments I made on the code and fix the CI errors. Will then follow-up with a more complete review.

Yes, I will finish this issue before moving to another. For a reason, I hadn't noticed your new comments and I was searching for another issue while I was waiting.

github-actions · 2026-01-25T12:14:20Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

github-actions · 2026-01-26T06:04:07Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

github-actions · 2026-01-28T10:14:12Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0
  Reference: TOTAL: 64 / PASSED: 15 / FAILED: 49 / SKIPPED: 0

LoukasPap · 2026-03-14T16:08:55Z

Returning to that issue, are there any other changes to be made? Or, is the approach to the problem "correct"? Maybe simpler/cleaner solutions exist.

src/sed/compiler.rs

README.md

src/sed/compiler.rs

dspinellis · 2026-03-14T16:25:53Z

@sylvestre I think we were reviewing concurrently. Would the proposed scheme for a single field address your concerns? What's your opinion regarding it?

sylvestre · 2026-03-14T16:34:26Z

sure, sounds good :)

…based on POSIX flag - Update `get_cmd_spec()` and `get_verified_cmd_spec()` to accept a boolean POSIX param - Add unit and high level tests - Update README.md

LoukasPap force-pushed the main branch 2 times, most recently from 81e0901 to 698f2b7 Compare January 21, 2026 17:07

LoukasPap commented Jan 21, 2026

View reviewed changes

src/sed/compiler.rs Outdated Show resolved Hide resolved

LoukasPap force-pushed the main branch from 698f2b7 to dd23a6f Compare January 21, 2026 17:23

LoukasPap mentioned this pull request Jan 22, 2026

Fix address ranges for i and a commands #261

Closed

LoukasPap force-pushed the main branch 3 times, most recently from d980254 to a73f56d Compare January 22, 2026 17:18

dspinellis reviewed Jan 22, 2026

View reviewed changes

src/sed/command.rs Outdated Show resolved Hide resolved

dspinellis requested changes Jan 22, 2026

View reviewed changes

src/sed/compiler.rs Outdated Show resolved Hide resolved

LoukasPap changed the title ~~Fix address range difference for line number command~~ Add support for address range (GNU extension) Jan 24, 2026

LoukasPap changed the title ~~Add support for address range (GNU extension)~~ Add support for address range commands (GNU extension) Jan 24, 2026

LoukasPap force-pushed the main branch from a73f56d to 4b76a36 Compare January 25, 2026 11:51

LoukasPap force-pushed the main branch from 4b76a36 to 06b90cc Compare January 25, 2026 21:47

LoukasPap requested a review from dspinellis January 25, 2026 22:05

sylvestre force-pushed the main branch from 06b90cc to 1e3638c Compare January 28, 2026 07:41

sylvestre force-pushed the main branch from 1e3638c to ff0dbc2 Compare February 2, 2026 08:07

LoukasPap force-pushed the main branch from ecbf826 to 5494272 Compare March 14, 2026 15:55

LoukasPap marked this pull request as draft March 14, 2026 15:55

LoukasPap marked this pull request as ready for review March 14, 2026 16:00

dspinellis requested changes Mar 14, 2026

View reviewed changes

src/sed/compiler.rs Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

src/sed/compiler.rs Show resolved Hide resolved

sylvestre reviewed Mar 14, 2026

View reviewed changes

src/sed/compiler.rs Outdated Show resolved Hide resolved

sylvestre reviewed Mar 14, 2026

View reviewed changes

src/sed/compiler.rs Outdated Show resolved Hide resolved

sylvestre reviewed Mar 14, 2026

View reviewed changes

src/sed/compiler.rs Outdated Show resolved Hide resolved

Set max addresses to 2 for a, i and = and make n_addr conditional, …

941c695

…based on POSIX flag - Update `get_cmd_spec()` and `get_verified_cmd_spec()` to accept a boolean POSIX param - Add unit and high level tests - Update README.md

LoukasPap force-pushed the main branch from 5494272 to 941c695 Compare March 15, 2026 20:23

LoukasPap requested review from dspinellis and sylvestre March 16, 2026 10:23

Conversation

LoukasPap commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 12, 2026

Uh oh!

codecov bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jan 16, 2026

Uh oh!

dspinellis commented Jan 20, 2026

Uh oh!

LoukasPap commented Jan 20, 2026

Uh oh!

LoukasPap commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dspinellis commented Jan 20, 2026

Uh oh!

github-actions bot commented Jan 21, 2026

Uh oh!

github-actions bot commented Jan 21, 2026

Uh oh!

github-actions bot commented Jan 21, 2026

Uh oh!

LoukasPap commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 21, 2026

Uh oh!

github-actions bot commented Jan 22, 2026

Uh oh!

dspinellis commented Jan 22, 2026

Uh oh!

codspeed-hq bot commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

LoukasPap commented Jan 22, 2026

Uh oh!

Uh oh!

dspinellis commented Jan 22, 2026

Uh oh!

dspinellis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LoukasPap commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 25, 2026

Uh oh!

github-actions bot commented Jan 26, 2026

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

LoukasPap commented Mar 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dspinellis commented Mar 14, 2026

Uh oh!

sylvestre commented Mar 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LoukasPap commented Jan 12, 2026 •

edited

Loading

codecov bot commented Jan 12, 2026 •

edited

Loading

LoukasPap commented Jan 20, 2026 •

edited

Loading

LoukasPap commented Jan 21, 2026 •

edited

Loading

codspeed-hq bot commented Jan 22, 2026 •

edited

Loading

LoukasPap commented Jan 24, 2026 •

edited

Loading