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

Strict mode #7

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Strict mode #7

merged 2 commits into from
Sep 23, 2024

Conversation

seddonym
Copy link
Collaborator

@seddonym seddonym commented Sep 23, 2024

Adds an optional strict parameter to the Bundle constructor, allowing client code to decide whether an exception should be raised if the fluent file contains errors.

I did hope to add more structured information to the ParserError, so we could see which lines had errors in, but I couldn't quickly work out how to define exceptions in Py03 that have custom fields. (In its current form, the string-based reporting is too long to read, IMO).

I think it's probably worth merging like this as-is and then tackle better error reporting in a follow up, but open to being talked out of that! 😄

To give client code more control about what exceptions it catches.
@@ -37,8 +42,9 @@ impl Bundle {
Ok(resource) => resource,
Err(error) => {
if strict {
return Err(PyValueError::new_err(format!(
"{error:?} - Fluent file contains errors"
Copy link
Collaborator Author

@seddonym seddonym Sep 23, 2024

Choose a reason for hiding this comment

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

The debug output of the error class was really long and difficult to read. Ideally we would have an errors attribute on the Python exception which contains each error as structured data. Happy to use the original error message for the time being though if you feel it's better.

@seddonym seddonym marked this pull request as ready for review September 23, 2024 14:02
@seddonym seddonym requested a review from a team as a code owner September 23, 2024 14:02
@seddonym seddonym requested a review from dooferlad September 23, 2024 14:02
@seddonym seddonym merged commit 39014d0 into main Sep 23, 2024
1 check passed
@seddonym seddonym deleted the strict-mode branch September 23, 2024 16:08
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