Skip to content

Consider switching format_pattern to (String, Vec<Error>) #125

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

Closed
zbraniecki opened this issue Jul 23, 2019 · 7 comments
Closed

Consider switching format_pattern to (String, Vec<Error>) #125

zbraniecki opened this issue Jul 23, 2019 · 7 comments
Labels
crate:fluent-bundle Issues related to fluent-bundle crate design-decision Issues pending design decision, usually Rust specific good first issue Want to help? Those are great bugs to start with! help wanted We need help making decisions or writing PRs for this. question
Milestone

Comments

@zbraniecki
Copy link
Collaborator

Are you still planning to change the return value of format_pattern to (String, Option<Vec<Error>>)?

Originally posted by @stasm in #120

@zbraniecki zbraniecki added this to the 0.7 milestone Jul 23, 2019
@stasm
Copy link
Contributor

stasm commented Jul 24, 2019

Related: we could also consider removing errors from the signature of format_pattern.

@zbraniecki
Copy link
Collaborator Author

Related: we could also consider removing errors from the signature of format_pattern.

What do you mean by that?

@stasm
Copy link
Contributor

stasm commented Jul 24, 2019

Since errors would be returned from format_pattern (wrapped in an Option), I think there's little use left for the third argument to format_pattern. We could thus change:

let value = bundle.format_pattern(&pattern, Some(&args), &mut errors);

to:

let (value, maybe_errors) = bundle.format_pattern(&pattern, Some(&args));

@zbraniecki
Copy link
Collaborator Author

Ah yes, that was my thinking as well!

@zbraniecki
Copy link
Collaborator Author

This gets moved to 0.8.

@zbraniecki zbraniecki modified the milestones: 0.7, 0.8 Aug 1, 2019
@zbraniecki zbraniecki modified the milestones: 0.10, 0.11 Dec 29, 2019
@zbraniecki zbraniecki added design-decision Issues pending design decision, usually Rust specific crate:fluent-bundle Issues related to fluent-bundle crate good first issue Want to help? Those are great bugs to start with! help wanted We need help making decisions or writing PRs for this. question labels Jun 16, 2020
@zbraniecki
Copy link
Collaborator Author

I actually went the other direction - I now accept a &Vec of Errors that FluentBundle populates. I think this is a better approach as it allows the user to decide how to handle errors (per format, or per list of formats) without any additional allocations.

I may also switch to allow not to pass the errors in which case errors won't be populated at all.

@zbraniecki
Copy link
Collaborator Author

I'm going to close it. I believe that errors vec should not be allocated as part of the return value, and instead should be passed to the call in some higher level code that will handle the outcome of errors (fallbacks, reporting etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate:fluent-bundle Issues related to fluent-bundle crate design-decision Issues pending design decision, usually Rust specific good first issue Want to help? Those are great bugs to start with! help wanted We need help making decisions or writing PRs for this. question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants