Skip to content

find: point at the offending argument in expression errors - #874

Open
sylvestre wants to merge 6 commits into
uutils:mainfrom
sylvestre:find-diagnostics
Open

sylvestre wants to merge 6 commits into
uutils:mainfrom
sylvestre:find-diagnostics

Conversation

@sylvestre

@sylvestre sylvestre commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

When a find expression is long, a single error line doesn't say where the problem is. This series makes expression parse errors record the argument they refer to. When stderr is a terminal, or when UUTILS_DIAG=always is set, find then underlines that argument on the echoed command line and adds a "did you mean" suggestion for a mistyped predicate:

$ find /srv -type f -a \( -name '*.rs' -o -nmae '*.toml' \) -print
find: unknown predicate `-nmae'
   ╭─[ find:1:38 ]
   │
 1 │ find /srv -type f -a ( -name *.rs -o -nmae *.toml ) -print
   │                                      ──┬──
   │                                        ╰──── not a known predicate
   │
   │ Help: did you mean `-name'?
───╯

When stderr is piped or redirected (scripts, the GNU and bfs testsuites), output is still the single line, byte-for-byte what GNU prints. UUTILS_DIAG=never forces that everywhere.

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

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 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.74041% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.40%. Comparing base (ac9cc79) to head (4c15fdb).

Files with missing lines Patch % Lines
src/find/matchers/mod.rs 85.78% 26 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   92.22%   92.40%   +0.18%     
==========================================
  Files          35       36       +1     
  Lines        7524     7691     +167     
  Branches      390      397       +7     
==========================================
+ Hits         6939     7107     +168     
+ Misses        443      442       -1     
  Partials      142      142              

☔ 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 27, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing sylvestre:find-diagnostics (4c15fdb) with main (ac9cc79)

Open in CodSpeed

@github-actions

Copy link
Copy Markdown

Commit 603c919 has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 315 / PASSED: 272 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 317 / PASSED: 274 / FAILED: 37 / SKIPPED: 6

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

No result in this run (2) - hung, crashed, or renamed:
  ? gnu/execdir_plus_semicolon (was PASS)
  ? posix/flag_weird_names (was PASS)

@github-actions

Copy link
Copy Markdown

Commit d68c009 has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 316 / PASSED: 273 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 317 / PASSED: 274 / FAILED: 37 / SKIPPED: 6

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

No result in this run (1) - hung, crashed, or renamed:
  ? posix/flag_weird_names (was PASS)

Copilot AI review requested due to automatic review settings September 27, 2026 10:15

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.

Quote the predicate or operator as GNU 4.10.0 does, report an operator
cut off by ')' and an unmatched ')' with GNU's messages, and name a
binary operator as spelled.
Lets an unclosed '(' be pointed at later. No behaviour change.
Add a `ParseError` carrying the argument index and a label. Its Display
is the bare message, so output is unchanged.
Copilot AI review requested due to automatic review settings September 27, 2026 15:37

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.

@github-actions

Copy link
Copy Markdown

Commit 4112abc has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 317 / PASSED: 274 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 316 / PASSED: 273 / FAILED: 37 / SKIPPED: 6

Changes from main branch:
  TOTAL: +1
  PASSED: +1
  FAILED: +0

The suggestion travels as help text, so the message stays GNU's.

The parser now matches on a `Predicate` looked up in a single name
table, which also supplies the suggestions: a table entry the parser
does not handle fails the exhaustive match, and a variant missing from
the table is flagged as never constructed.
Underline the offending argument through `uucore::diagnostics` when
stderr is a terminal or `UUTILS_DIAG=always`. The plain line still
leads, and captured output stays the single GNU line.

The report heading comes from uucore's `util_name()`, which keeps the
`.exe` on Windows; the plain line keeps `program_name()` like every
other message.
Copilot AI review requested due to automatic review settings September 27, 2026 17:28

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.

@github-actions

Copy link
Copy Markdown

Commit 4c15fdb has test result changes:

bfs testsuite:

Test results comparison:
  Current:   TOTAL: 316 / PASSED: 273 / FAILED: 37 / SKIPPED: 6
  Reference: TOTAL: 316 / PASSED: 273 / FAILED: 37 / SKIPPED: 6

No result in this run (1) - hung, crashed, or renamed:
  ? gnu/executable (was PASS)

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