Add support for address range commands (GNU extension) #260

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

Add GNU extension for 2 supported addresses for a, i and = commands

UPDATE
The PR now solves the problem where invalid POSIX commands with address range were being allowed when they shouldn't. These commands are: a, i, =, l, q, r (maybe there are some more I missed).

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 75.60976% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.97%. Comparing base (a6207cb) to head (ff0dbc2).

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #260      +/-   ##
==========================================
- Coverage   82.03%   81.97%   -0.07%     
==========================================
  Files          13       13              
  Lines        5423     5454      +31     
  Branches      291      292       +1     
==========================================
+ Hits         4449     4471      +22     
- Misses        972      981       +9     
  Partials        2        2

Flag	Coverage Δ
macos_latest	`82.41% <75.60%> (-0.07%)`	⬇️
ubuntu_latest	`82.53% <75.60%> (-0.07%)`	⬇️
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

CodSpeed Performance Report

Merging this PR will not alter performance

_{Comparing LoukasPap:main (ff0dbc2) with main (a6207cb)}

Summary

✅ 11 untouched benchmarks

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

…flag usage with GNU-only extensions - Update check for invalid address range in POSIX mode by adding `n_add_posix` field in CommandSpec struct which specifies what is the number of addresses allowed for each command in POSIX mode - Add integration and unit 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

Add support for address range commands (GNU extension) #260

Are you sure you want to change the base?

Add support for address range commands (GNU extension) #260

Uh oh!

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!

CodSpeed Performance Report

Merging this PR will not alter performance

Summary

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 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