projectfluent / fluent-rs

Rust implementation of Project Fluent
https://projectfluent.org
Apache License 2.0
1.04k stars 95 forks source link

Replace cases of `&mut Vec<Error>` with lazy error iteration #254

Closed clarfonthey closed 2 months ago

clarfonthey commented 2 years ago

Right now, APIs like format_message will just add to a list of localisation errors, which really sucks from multiple usability standpoints:

  1. It's extremely un-rusty for an API, since an error occurring will still return a string, even if that string is ill-formed.
  2. The user has to wait for all errors to be registered and stored in the vec, instead of being able to just take the first one.
  3. The only feasible ways of using this kind of API across a program are with global state (bad) or allocating a vec on every usage (also bad).

My proposal is instead to make these APIs return a type along the lines of Result<Cow, ErrorIter>, where ErrorIter is an iterator over the errors found by fluent. This iterator can either be added to a vec directly, or passed on through multiple callers so that eventually it reaches one who is in charge of displaying errors.

Additionally, users can take advantage of iterator adapters by chaining multiple ErrorIter if they want to return more, or they can choose to take the easy route and just return a Vec in these cases.

Basically, this puts control back into the user of the API, rather than making a decision on how they should handle errors for them.

gregtatum commented 1 year ago

This seems like a reasonable approach to me. It'll be a bit of a big API change for consumers. I know Firefox would need to adjust our integration for it with the change.

alerque commented 2 months ago

I'm going to close this as a duplicate of #323, although the immediate problem is not the same. The underlying API surface is the same, and if any refactoring is done that would change the API at all I think whatever solution is used should address both problems at once. I've copied the comments above to that discussion so they don't get lost if work is done from the concurrency angle of things.