Skip to content
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

Expose better errors #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Waelwindows
Copy link

The previous errors didn't allow API consumers to inspect the underlying cause for their error. Whether it was to handle the error manually or to display a proper "error-chain" for the errors (à la eyre, anyhow, etc).

The previous errors didn't allow API consumers to inspect the underlying
cause for their error. Whether it was to handle the error manually or to
display a proper "error-chain" for the errors (à la `eyre`, `anyhow`, etc).
@@ -0,0 +1 @@
use_flake
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this file and the flake files.

@@ -1,3 +1,7 @@
*~
target
Cargo.lock

# Nix
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather keep environment-specific config out of this repository. You can always add these to your global gitignore. See https://git-scm.com/docs/gitignore. It says the default location is $XDG_CONFIG_HOME/git/ignore or if $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore.

/// Underlying errors for [`PropertiesError`]
#[non_exhaustive]
#[derive(Debug, Error)]
pub enum PropertiesErrorCause {
Copy link
Owner

Choose a reason for hiding this comment

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

It doesn't make sense for this struct to implement Error as well. Having separate IOError, EncodingError, etc. cases doesn't provide much benefit, because the user could always just call source() and downcast.

It probably makes more sense to create BadKeyValueSeparator and BadCommentPrefix structs, implement Error for them, make all of these direct causes of PropertiesError, and remove the PropertiesErrorCause struct.

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