Skip to content

sed: implement modern diagnostic like coreutils - #576

Merged
sylvestre merged 1 commit into
mainfrom
modern-diag
Sep 27, 2026
Merged

sylvestre merged 1 commit into
mainfrom
modern-diag

Conversation

@sylvestre

@sylvestre sylvestre commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI lite review requested due to automatic review settings September 26, 2026 17:55
@sylvestre
sylvestre added this pull request to stack #577 September 26, 2026 17:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 26, 2026 18:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 26, 2026 18:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.82609% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.71%. Comparing base (72ed709) to head (c099152).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/sed/error_handling.rs 75.00% 20 Missing and 1 partial ⚠️
src/sed/script_char_provider.rs 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #576      +/-   ##
==========================================
- Coverage   83.87%   83.71%   -0.17%     
==========================================
  Files          14       14              
  Lines        6977     7060      +83     
  Branches      407      413       +6     
==========================================
+ Hits         5852     5910      +58     
- Misses       1121     1145      +24     
- Partials        4        5       +1     
Flag Coverage Δ
macos_latest 84.84% <72.82%> (-0.19%) ⬇️
ubuntu_latest 85.04% <72.82%> (-0.19%) ⬇️
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 Harness.
📢 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.

@codspeed

codspeed Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.69%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 8 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ number_fix 1.1 s 1.3 s -11.02%
❌ access_log_translit 1 s 1 s -3.17%
⚡ access_log_subst 2.4 s 2.3 s +3.69%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing modern-diag (c099152) with main (72ed709)

Open in CodSpeed

Base automatically changed from mdbook to main September 27, 2026 09:42
@sylvestre sylvestre changed the title sed: underline the offending script character at a terminal sed: implement modern diagnostic like coreutils Sep 27, 2026
Copilot AI review requested due to automatic review settings September 27, 2026 15:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 27, 2026 16:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

When stderr is a terminal (or UUTILS_DIAG=always), quote the offending
script line and underline the faulty character, in the layout uucore's
diagnostics use. Pipes and the test suite still get the single-line
message.

Both error constructors handle this, so no call site changes.
ScriptLocation now carries the line text, captured only when diagnostics
are on and shared between commands from the same line, so semantic
errors raised after compilation can still be drawn.

Anything that cannot be drawn (non-UTF-8 script, empty line, no line
recorded) leaves the message unchanged.
Copilot AI review requested due to automatic review settings September 27, 2026 18:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sylvestre
sylvestre merged commit a05d743 into main Sep 27, 2026
46 of 49 checks passed
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