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

Add error types, stop returning strings #2

Merged
merged 2 commits into from
Jan 6, 2025
Merged

Conversation

Minoru
Copy link
Member

@Minoru Minoru commented Jan 2, 2025

In Newsboat, we relay the error to the user, but other people might want to do other things. This commit replaces Strings with two new types, one for new() and one for matches(). Even though they are identical, I decided to have two of them because in the future they might become slightly different. (I don't foresee that, as the underlying API has been stable for decades, but if it ever happens I'd be glad I did it this way.)

As a result, we can drop dependencies on gettext-rs and strprintf.

@dennisschagt, @exaroth, I asked you for review primarily because I want your opinion on the names I picked :) I'd also appreciate feedback on the shape of the API; can you imagine using this outside Newsboat?

In Newsboat, we relay the error to the user, but other people might want
to do other things. This commit replaces `String`s with two new types,
one for `new()` and one for `matches()`. Even though they are identical,
I decided to have two of them because in the future they might become
slightly different. (I don't foresee that, as the underlying API has
been stable for decades, but if it ever happens I'd be glad I did it
this way.)

As a result, we can drop dependencies on `gettext-rs` and `strprintf`.
@Minoru Minoru requested review from exaroth and dennisschagt January 2, 2025 15:54
Copy link
Member

@dennisschagt dennisschagt left a comment

Choose a reason for hiding this comment

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

Nice improvements, LGTM

src/lib.rs Outdated Show resolved Hide resolved
@Minoru Minoru merged commit f120f9c into master Jan 6, 2025
@Minoru Minoru deleted the feature/add-error-types branch January 6, 2025 11:20
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