Repository navigation
uucore: draw diagnostics against any line of text - #15532
Conversation
Add SourceLine, for utilities whose instructions come from a script or a pattern file rather than the command line (sed, grep), and share the ariadne drawing with Snapshot. Also make use_color() public, decide enabled() once, and keep ariadne's "Help" when no localizer is set up.
There was a problem hiding this comment.
🟡 Changes recommended
The stub’s lifetime mismatch breaks feature-disabled callers, and rendering allocates memory proportional to the source line number.
2 open issues
What changed in this PR
Extends uucore’s shared caret diagnostics to named source lines, supporting script and pattern-file errors outside command-line arguments.
Changes:
- Adds
SourceLinerendering and a feature-disabled stub. - Shares rendering logic and exposes color detection.
- Caches diagnostic enablement and preserves “Help” without a localizer.
| File | Description |
|---|---|
| src/uucore/src/lib/features/diagnostics.rs | Adds source-line reports, shared rendering, and regression tests. |
| src/uucore/src/lib/features/diagnostics_stub.rs | Adds no-op source-line and color APIs. |
Review effort: Balanced
| let padding = self.line_number - 1; | ||
| let text = format!("{}{} ", "\n".repeat(padding), self.text); |
7009cae to
7ed959c
Compare
| help: Option<&str>, | ||
| color: bool, | ||
| ) -> Option<String> { | ||
| if self.line_number == 0 || self.text.is_empty() { |
| let open = header.find("[ ")? + 2; | ||
| let close = header.rfind(" ]")?; |
Merging this PR will degrade performance by 11.61%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | true_consecutive_calls |
294.7 ns | 350.2 ns | -15.86% |
| ❌ | Memory | du_deep_tree[(100, 3)] |
76.5 KB | 82.4 KB | -7.14% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing sylvestre:uucore-diag-source-line (2dbcf99) with main (5fda18e)2
Footnotes
-
70 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(865e575) during the generation of this report, so 5fda18e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
7ed959c to
2dbcf99
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
4 open issues
Review effort: Lite
| /// Whether a drawn report is colored: at a terminal, unless `NO_COLOR` is set. | ||
| pub fn use_color() -> bool { | ||
| env::var_os("NO_COLOR").is_none() && std::io::stderr().is_terminal() | ||
| } |
The caret diagnostics only know how to point into the command line. sed, grep and findutils want the same reports, but sed's script errors and grep's `-f` patterns live in a script or a file, not in argv, so sed carried its own copy of the layout.