diagnostics: point inside quoted arguments, share the locating

[sylvestre]

Three pieces of sharing. An argument holding a space is echoed quoted, so an
offset inside it no longer lands where the operand ends — the caret now follows
the operand's bytes wherever the quoting put them. The expression was already complete label and the locating of an option's value both move to one place
instead of being spelled out per utility.

• diagnostics: share the "expression was already complete" label
• diagnostics: point inside an argument the report had to quote
• uucore: share the locating of an option's value

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)

[codspeed-hq]

Merging this PR will improve performance by 7.07%

⚡ 6 improved benchmarks
✅ 347 untouched benchmarks
⏩ 50 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Simulation	cksum_crc	41.5 ms	38.2 ms	+8.56%
⚡	Simulation	cksum_raw_output	41.4 ms	38.2 ms	+8.37%
⚡	Simulation	cksum_multiple_files	63.1 ms	58.3 ms	+8.22%
⚡	Simulation	cksum_crc32b	41.4 ms	38.7 ms	+6.89%
⚡	Simulation	cksum_default	41.4 ms	38.8 ms	+6.88%
⚡	Simulation	wc_bytes_synthetic[1]	187.9 µs	181.4 µs	+3.61%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing diag-shared-locating (cc947ba) with diag-localization (bb702f0)}

Footnotes

1. 50 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. ↩

[Copilot]

Pull request overview

This PR improves caret-style diagnostics rendering so underline/caret positions remain accurate even when arguments are echoed back with quoting, and it centralizes two previously duplicated diagnostic behaviors (the “expression already complete” label and option-value locating/rendering).

Changes:

• Update snapshot locating so offsets inside an operand still map correctly when the argument is quoted (e.g., contains spaces), and refine fallback behavior at operand boundaries.
• Introduce shared boundary/character-span helpers and a shared Snapshot::render_option_value(...) helper to reduce per-utility duplication.
• Consolidate the “expression was already complete here” diagnostic label into uucore locales and reuse it from test and expr.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/by-util/test_printf.rs	Updates the expected caret column/underline for a quoted format-string argument.
src/uucore/src/lib/features/mode.rs	Switches mode diagnostics rendering to the shared option-value renderer and factors span/label/help derivation.
src/uucore/src/lib/features/diagnostics.rs	Implements shared render_option_value, improves locating within quoted args, and re-exports boundary helpers.
src/uucore/src/lib/features/diagnostics_stub.rs	Mirrors the new API (re-exports boundary helpers and adds render_option_value) for builds without the diagnostics feature.
src/uucore/src/lib/features/diagnostics_boundary.rs	New shared module for character-boundary flooring and single-character span calculation (with unit tests).
src/uucore/src/lib/features.rs	Adds the shared diagnostics_boundary module used by both real and stub diagnostics implementations.
src/uucore/locales/en-US.ftl	Adds shared diagnostics label diagnostics-label-expression-complete.
src/uucore/locales/fr-FR.ftl	Adds shared diagnostics label diagnostics-label-expression-complete.
src/uu/test/src/diagnostics.rs	Reuses the shared “expression complete” label key from uucore.
src/uu/test/locales/en-US.ftl	Removes the now-duplicated per-utility label key.
src/uu/test/locales/fr-FR.ftl	Removes the now-duplicated per-utility label key.
src/uu/expr/src/diagnostics.rs	Reuses the shared “expression complete” label key from uucore.
src/uu/expr/locales/en-US.ftl	Removes the now-duplicated per-utility label key.
src/uu/expr/locales/fr-FR.ftl	Removes the now-duplicated per-utility label key.

---

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

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/uucore/src/lib/features/diagnostics.rs:436

• The docstring states that an empty range marks the character it starts at, but the implementation now falls back to underlining the whole operand when the empty range is at the end (see a_range_at_the_end_of_an_operand_falls_back_to_the_operand). Please update the docs to describe the end-of-operand behavior (or adjust behavior to match the docs) so callers have an accurate contract.

    /// * `range` - Byte range inside `operand` to point at. An empty range
    ///   marks the character it starts at.

src/uucore/locales/en-US.ftl:115

• A new shared localization key is introduced in uucore and is now referenced by multiple utilities, but it’s only added for en-US and fr-FR in this PR. If the project ships additional uucore locale files, those locales will lack this key and may produce fallback/blank strings or missing-key warnings at runtime. Consider adding this key to the remaining uucore locales (even if temporarily in English) to keep localization behavior consistent.

# Diagnostic label shared by the utilities whose arguments are an expression
diagnostics-label-expression-complete = the expression was already complete here

src/uucore/src/lib/features/diagnostics_boundary.rs:30

• floor_boundary currently scans from offset down to 0 using an iterator, which is O(n) with some iterator overhead per call. Since diagnostics may call this repeatedly, consider using a simple decrementing loop (e.g., start at offset.min(text.len()), decrement until is_char_boundary) to reduce overhead while keeping the same semantics.

pub fn floor_boundary(text: &str, offset: usize) -> usize {
    let offset = offset.min(text.len());
    (0..=offset)
        .rev()
        .find(|&i| text.is_char_boundary(i))
        .unwrap_or(0)
}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/uucore/src/lib/features/diagnostics.rs:436

• The docs for range no longer match the updated behavior in locate_operand: an empty range at the end of the operand falls back to underlining the whole operand (per the updated tests), not “the character it starts at” (there is no character). Update this doc comment to describe the end-of-operand behavior explicitly (e.g., “empty range marks the character it starts at; if it starts at/after the end of the operand, the whole operand is underlined”).

    /// * `range` - Byte range inside `operand` to point at. An empty range
    ///   marks the character it starts at.

src/uucore/src/lib/features/mode.rs:92

• self.describe() performs translations unconditionally, even though render_option_value can return false early when index_of_value can’t find the option value. Previously, that translation work happened only after finding the value. Consider making translation lazy here (e.g., first find the index/value location, then translate and call render_inside_at), or adjusting render_option_value to accept label/help as deferred computation so translations only happen when rendering proceeds.

        let (label, help) = self.describe();
        crate::diagnostics::Snapshot::new(args).render_option_value(
            mode,
            Some('m'),
            Some("mode"),
            self.clause_span(clause_start),
            message,
            label.as_deref(),
            help.as_deref(),
        )

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/uucore/src/lib/features/mode.rs:92

• ModeError::rendernow translateslabel/helpviadescribe()even whenrender_option_valuewill immediately returnfalse(e.g., diagnostics compiled out via the stub, or when the option/value isn’t found). Previously this path could avoid translation work by checkingindex_of_valuefirst. Consider restructuring to only translate after confirming the value is present (e.g., explicitly dolet snap = Snapshot::new(args); let Some(index) = snap.index_of_value(...) else { return false; };then callrender_inside_at, or introduce a variant of render_option_value` that takes untranslated keys / a lazy translation callback).

        let (label, help) = self.describe();
        crate::diagnostics::Snapshot::new(args).render_option_value(
            mode,
            Some('m'),
            Some("mode"),
            self.clause_span(clause_start),
            message,
            label.as_deref(),
            help.as_deref(),
        )

src/uucore/src/lib/features/diagnostics.rs:436

• The doc for range says an empty range “marks the character it starts at”, but the current behavior falls back to underlining the whole operand when the empty range starts at the end of the operand (see a_range_at_the_end_of_an_operand_falls_back_to_the_operand). Please update the doc comment to reflect the end-of-operand case (e.g., “marks the character it starts at, or the whole operand if it starts at the end”).

    /// * `range` - Byte range inside `operand` to point at. An empty range
    ///   marks the character it starts at.