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

Remove a #[allow(clippy::match_wild_err_arm)] and dedup error message creation #46

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

Kijewski
Copy link
Collaborator

@Kijewski Kijewski commented Jul 4, 2024

No description provided.

Ok(source)
} else {
let msg = format!(
"unable to open template file '{}'",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should provide the Err() value in the error message, it's not always obvious what's wrong (like wrong rights).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call! Added.

@Kijewski Kijewski force-pushed the pr-one-less-allow branch from 6abf376 to ddd4a81 Compare July 4, 2024 09:00
@@ -0,0 +1,10 @@
error: unable to open template file '$WORKSPACE/target/tests/trybuild/rinja_testing/templates/a_file_that_is_actually_a_folder.html':
Is a directory (os error 21)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hum... I think having the error text on the same line might be better. What do you think?

PS: Having a test for this is great, thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure if the line does not become too long, then. But chances are that you aren't using such a long path in a real project. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Boarf, I think it's fine. I find it less confusing this way.

@Kijewski Kijewski force-pushed the pr-one-less-allow branch from ddd4a81 to 6a69946 Compare July 4, 2024 09:03
@GuillaumeGomez
Copy link
Contributor

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit e3c1fe3 into rinja-rs:master Jul 4, 2024
17 checks passed
@Kijewski Kijewski deleted the pr-one-less-allow branch July 4, 2024 09:09
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