Improve GNU sed testsuite compatibility by kevinburkesegment · Pull Request #345 · uutils/sed

kevinburkesegment · 2026-03-19T22:43:36Z

Fix l command default width from 60 to 70 (matching GNU sed)
Wire -l flag and COLS env var to l command width
Reject l<N> (GNU extension) in --posix mode
Accept } and # as valid command terminators (fixes command-endings)
Accept } and # as valid substitute flag terminators
Eat spaces before command endings (e.g. y/1/a/ ;d)
Scope #n silent mode to first script source only (fixes comment-n)
Drop backslash for unknown escape sequences in s/// replacement (GNU behavior)
Match GNU sed error message format: -e expression #N, char C: for string scripts, file:line: for file scripts
Add get_source_index() to ScriptLineProvider
Add test-results.txt to .gitignore

GNU testsuite results: 7 → 9 passing (command-endings, comment-n now pass).

github-actions · 2026-03-19T22:46:38Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 9 / FAILED: 46 / SKIPPED: 10
  Reference: TOTAL: 65 / PASSED: 7 / FAILED: 48 / SKIPPED: 10

Changes from main branch:
  TOTAL: +0
  PASSED: +2
  FAILED: -2

Test improvements (2):
  + command-endings
  + comment-n

github-actions · 2026-03-19T22:49:08Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 9 / FAILED: 46 / SKIPPED: 10
  Reference: TOTAL: 65 / PASSED: 7 / FAILED: 48 / SKIPPED: 10

Changes from main branch:
  TOTAL: +0
  PASSED: +2
  FAILED: -2

Test improvements (2):
  + command-endings
  + comment-n

codecov · 2026-03-19T22:50:53Z

Codecov Report

❌ Patch coverage is 71.18644% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.92%. Comparing base (7fade42) to head (e73ac32).

Files with missing lines	Patch %	Lines
src/sed/compiler.rs	65.62%	11 Missing ⚠️
src/sed/error_handling.rs	66.66%	5 Missing ⚠️
src/sed/script_line_provider.rs	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
- Coverage   82.16%   81.92%   -0.25%     
==========================================
  Files          13       13              
  Lines        5444     5454      +10     
  Branches      304      312       +8     
==========================================
- Hits         4473     4468       -5     
- Misses        969      984      +15     
  Partials        2        2

Flag	Coverage Δ
macos_latest	`82.36% <71.18%> (-0.25%)`	⬇️
ubuntu_latest	`82.47% <71.18%> (-0.25%)`	⬇️
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.

sylvestre · 2026-03-20T06:55:02Z

Sorry but only one commit implementating all the changes isn't the best practice. In a perfect world, one pr changes or at least one commit per fix in this pr

sylvestre · 2026-03-20T06:55:25Z

For example, it makes it hard to know what is cover by an existing test or not

github-actions · 2026-04-02T16:24:27Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 9 / FAILED: 46 / SKIPPED: 10
  Reference: TOTAL: 65 / PASSED: 7 / FAILED: 48 / SKIPPED: 10

Changes from main branch:
  TOTAL: +0
  PASSED: +2
  FAILED: -2

Test improvements (2):
  + command-endings
  + comment-n

codspeed · 2026-04-02T16:28:23Z

Merging this PR will not alter performance

⚠️

1 benchmark spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

access_log_append

✅ 11 untouched benchmarks

_{Comparing kevinburkesegment:test-compatibility (470fafb) with main (58608af)}

sylvestre · 2026-08-08T23:10:44Z

@kevinburkesegment could you please rebase it? thanks

sylvestre · 2026-10-10T14:11:50Z

are you still interested in this one? if not, i will close it, thanks

sylvestre · 2026-10-10T14:11:59Z

 ) -> UResult<()> {
-    if !line.eol() && line.current() == ';' {
-        line.advance();
+    line.eat_spaces();


please add tests for } / # right after a command, spaces before ;, and #n in a second -e

sylvestre · 2026-10-10T14:12:06Z

+/// GNU sed checks COLS env var first, then terminal width, then defaults to 70.
+/// COLS values <= 0 or non-numeric are ignored.
 fn output_width() -> usize {
+    if let Ok(cols) = std::env::var("COLS")


where does this COLS handling come from? i can't find it in the GNU manual.
please make sure it isn't derived from the GNU source

sylvestre · 2026-10-10T14:12:09Z

                                literal.push(decoded);
                            } else {
-                                literal.push('\\');
+                                // Unknown escape: drop the backslash (GNU behavior)


please add a test for this in tests/by-util/test_sed.rs

sylvestre · 2026-10-10T14:12:13Z

    match parse_number(lines, line, false)? {
        Some(n) => {
+            // 'l<N>' is a GNU extension, reject in POSIX mode
+            if cmd.code == 'l' && context.posix {


please add a test for --posix with l5

sylvestre · 2026-10-10T14:12:17Z

-    Err(USimpleError::new(
-        1,
+    let input_name = lines.get_input_name();
+    let message = if input_name.starts_with("-e expression") {


matching on starts_with("-e expression") 3 times is fragile and duplicated.
could ScriptLocation store the kind of source instead?

sylvestre · 2026-10-10T14:12:21Z

-        length: matches.get_one::<u32>("length").map_or(70, |v| *v as usize),
+        // 0 means "not explicitly set via -l"; the l command then
+        // falls back to COLS, terminal width, or DEFAULT_OUTPUT_WIDTH.
+        length: matches.get_one::<u32>("length").map_or(0, |v| *v as usize),


with this, -l 0 falls back to the terminal width, but in GNU it means never wrap.
please add a test

sylvestre · 2026-10-10T14:12:25Z

        .fails()
        .code_is(1)
-        .stderr_is("sed: :0:3: error: incomplete command\n");
+        .stderr_is("sed: :0: incomplete command\n");


sed: :0: incomplete command looks broken (empty name, line 0), could you please fix it rather than update the expectation?

sylvestre · 2026-10-10T14:12:32Z

        .fails()
        .code_is(2)
-        .stderr_is("sed: <script argument 1>:2:3: 'input/lines1':1 error: no previous regular expression\n");
+        .stderr_is("sed: -e expression #1, char 3: 'input/lines1':1 error: no previous regular expression\n");


we lose the line info here. GNU counts char N from the start of the whole expression, not from the start of the line, no?

- Fix `l` command default width from 60 to 70 (matching GNU sed) - Wire `-l` flag and COLS env var to `l` command width - Reject `l<N>` (GNU extension) in --posix mode - Accept `}` and `#` as valid command terminators (fixes command-endings) - Accept `}` and `#` as valid substitute flag terminators - Eat spaces before command endings (e.g. `y/1/a/ ;d`) - Scope `#n` silent mode to first script source only (fixes comment-n) - Drop backslash for unknown escape sequences in s/// replacement (GNU behavior) - Match GNU sed error message format: `-e expression #N, char C:` for string scripts, `file:line:` for file scripts - Add `get_source_index()` to ScriptLineProvider - Add test-results.txt to .gitignore GNU testsuite results: 7 → 9 passing (command-endings, comment-n now pass). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Diagnostic mismatches, a stale test, and width and source-numbering errors remain unresolved.

5 open findings

What changed in this PR

Improves GNU sed compatibility in script parsing, list-output widths, and diagnostic expectations.

Changes:

Adjusts command terminators, replacement escapes, and first-source #n handling.
Connects list width to -l and COLS, with a default of 70.
Renames expression sources and updates diagnostic documentation and assertions.

File	Description
tests/by-util/test_sed.rs	Updates diagnostic expectations.
src/sed/script_line_provider.rs	Exposes source indices and renames expression sources.
src/sed/mod.rs	Changes default list-width configuration.
src/sed/error_handling.rs	Documents intended GNU diagnostic formats.
src/sed/compiler.rs	Adjusts parsing, escapes, silent mode, and list widths.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+                                // Unknown escape: drop the backslash (GNU behavior)
                                literal.push(line.current_byte());
                                line.advance();


+/// Format matches GNU sed: `-e expression #N, char C: message` for string
+/// scripts, `file:line: message` for file scripts.


+        // Subtract 1 to avoid line wraps on terminals
+        return n - 1;


-        length: matches.get_one::<u32>("length").map_or(70, |v| *v as usize),
+        // 0 means "not explicitly set via -l"; the l command then
+        // falls back to COLS, terminal width, or DEFAULT_OUTPUT_WIDTH.
+        length: matches.get_one::<u32>("length").map_or(0, |v| *v as usize),


                    index: next_index,
                    reader: Box::new(BufReader::new(cursor)),
-                    input_name: format!("<script argument {}>", next_index + 1),
+                    input_name: format!("-e expression #{}", next_index + 1),


github-actions · 2026-10-15T17:14:02Z

GNU sed testsuite comparison:

Test results comparison:
  Current:   TOTAL: 65 / PASSED: 22 / FAILED: 35 / SKIPPED: 8
  Reference: TOTAL: 65 / PASSED: 18 / FAILED: 39 / SKIPPED: 8

Changes from main branch:
  TOTAL: +0
  PASSED: +4
  FAILED: -4

Test improvements (4):
  + 8to7
  + command-endings
  + comment-n
  + subst-options

sylvestre · 2026-10-18T08:16:25Z

a bunch of jobs are failing

kevinburkesegment force-pushed the test-compatibility branch from cf46e9e to 60beef3 Compare March 19, 2026 22:44

kevinburkesegment force-pushed the test-compatibility branch from 60beef3 to e391df1 Compare March 19, 2026 22:47

sylvestre force-pushed the test-compatibility branch from e391df1 to e73ac32 Compare April 2, 2026 16:22

sylvestre reviewed Oct 10, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 15, 2026 16:51

kevinburkesegment force-pushed the test-compatibility branch from e73ac32 to 470fafb Compare October 15, 2026 16:51

Copilot started reviewing on behalf of kevinburkesegment October 15, 2026 16:57

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

		/// Format matches GNU sed: `-e expression #N, char C: message` for string
		/// scripts, `file:line: message` for file scripts.

Conversation

kevinburkesegment commented Mar 19, 2026

Uh oh!

github-actions Bot commented Mar 19, 2026

Uh oh!

github-actions Bot commented Mar 19, 2026

Uh oh!

codecov Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sylvestre commented Mar 20, 2026

Uh oh!

sylvestre commented Mar 20, 2026

Uh oh!

github-actions Bot commented Apr 2, 2026

Uh oh!

codspeed Bot commented Apr 2, 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!

sylvestre commented Aug 8, 2026

Uh oh!

sylvestre commented Oct 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

github-actions Bot commented Oct 15, 2026

Uh oh!

sylvestre commented Oct 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Mar 19, 2026 •

edited

Loading

codspeed Bot commented Apr 2, 2026 •

edited

Loading