Skip to content

Improve GNU sed testsuite compatibility#345

Open
kevinburkesegment wants to merge 1 commit intouutils:mainfrom
kevinburkesegment:test-compatibility
Open

Improve GNU sed testsuite compatibility#345
kevinburkesegment wants to merge 1 commit intouutils:mainfrom
kevinburkesegment:test-compatibility

Conversation

@kevinburkesegment
Copy link
Copy Markdown
Contributor

  • 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
Copy link
Copy Markdown

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

- 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>
@github-actions
Copy link
Copy Markdown

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
Copy link
Copy Markdown

codecov bot commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 71.18644% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.92%. Comparing base (5e9b821) to head (e391df1).
⚠️ Report is 5 commits behind head on main.

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
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants