Skip to content

Move more errors to thiserror #7540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 30, 2025
Merged

Move more errors to thiserror #7540

merged 8 commits into from
Mar 30, 2025

Conversation

sgvictorino
Copy link
Contributor

@sgvictorino sgvictorino commented Mar 23, 2025

closes #7538, closes #7534, closes #7533, closes #7532, closes #7531, closes #7530, closes #7529

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/timeout/timeout is no longer failing!

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@sylvestre
Copy link
Contributor

in general, please try to avoid doing many changes in the same pr (and leave good first bug for others ;)
but not a big deal, could you just please remove the env change.

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@sylvestre sylvestre requested a review from Copilot March 30, 2025 18:46
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the error handling across multiple utilities by replacing custom Display and Error implementations with the thiserror derive macro. Key changes include:

  • Removing manual Error and Display impls and adding #[derive(Error)] with error attributes.
  • Updating error message formatting to provide more consistent and maintainable error messages.
  • Adding thiserror as a dependency to the affected Cargo.toml files.

Reviewed Changes

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

Show a summary per file
File Description
src/uu/wc/src/utf8/read.rs Migrate error definitions to use thiserror.
src/uu/unexpand/src/unexpand.rs Refactor ParseError to use thiserror with custom error messages.
src/uu/unexpand/Cargo.toml Add thiserror dependency.
src/uu/tac/src/error.rs Replace manual impls with thiserror-based error variants.
src/uu/tac/Cargo.toml Add thiserror dependency.
src/uu/split/src/strategy.rs Update NumberTypeError and StrategyError to use thiserror.
src/uu/split/src/split.rs Refactor SettingsError to use thiserror.
src/uu/split/src/filenames.rs Migrate SuffixError error handling to thiserror.
src/uu/split/Cargo.toml Add thiserror dependency.
src/uu/ptx/src/ptx.rs Update error variants to derive thiserror, removing Display impl.
src/uu/ptx/Cargo.toml Add thiserror dependency.
src/uu/numfmt/src/errors.rs Transition NumfmtError to use thiserror.
src/uu/numfmt/Cargo.toml Add thiserror dependency.
src/uu/expand/src/expand.rs Refactor ParseError to use thiserror attributes.
src/uu/expand/Cargo.toml Add thiserror dependency.
src/uu/dd/src/parseargs.rs Replace custom Display and Error impls with thiserror on ParseError.
src/uu/dd/Cargo.toml Add thiserror dependency.

@sylvestre sylvestre merged commit e20500d into uutils:main Mar 30, 2025
105 of 106 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
2 participants