Add FluentMultiLanguageLoader to allow easy translations access #62
Conversation
Signed-off-by: Martin <[email protected]>
Signed-off-by: Martin <[email protected]>
Signed-off-by: Martin <[email protected]>
|
hey @saskenuba thanks again for the PR, sorry I haven't reviewed it yet, I'm in the middle of moving countries so I don't have much spare time, but hopefully in about a week I can get to it. |
|
no problem @kellpossible! take a look when you have the time. |
i18n-embed/src/fluent/mod.rs
Outdated
| @@ -373,3 +326,68 @@ impl LanguageLoader for FluentLanguageLoader { | |||
| Ok(()) | |||
| } | |||
| } | |||
|
|
|||
| /// Retrieve and load the file for the designated `LanguageIdentifier` | |||
There was a problem hiding this comment.
the fallback_language variable is unused here, I think some of the original logic might have been been altered
There was a problem hiding this comment.
I can see the unused variable has been removed but I haven't spent the time to see how the error logic has been altered as a result of this.
Signed-off-by: Martin <[email protected]>
Signed-off-by: Martin <[email protected]>
Signed-off-by: Martin <[email protected]>
FluentMultiLang loader only errors out if the fallback language is missing, otherwise it skips to the next one. Added new error variant for this behaviour. FluentLang remains with the same behaviour as before. Signed-off-by: Martin <[email protected]>
|
Hello @kellpossible! Sorry, it took so long to reply to this PR. I followed your thoughts and I believe the API is simplified now. Can you review it again? Thanks! |
|
hey @saskenuba thanks! I'll take another look at it as soon as I get the time 🙂 |
| @@ -113,7 +118,7 @@ impl FluentLanguageLoader { | |||
| pub fn get_args_concrete<'source>( | |||
| &self, | |||
| message_id: &str, | |||
| args: HashMap<&'source str, FluentValue<'source>>, | |||
| args: HashMap<Cow<'source, str>, FluentValue<'source>>, | |||
There was a problem hiding this comment.
This is a breaking change (see fl_macro failing unit test), was there are particular motivation for using Cow over &str?
|
@saskenuba did you have a chance to look at this again? |
|
I'm in need for a working translation system that is able to switch the language between requests. Sometimes even within a request when generating multi-language responses. So this PR does exactly what I need. I'd like to jump in and complete this work if @kellpossible doesn't want to finish it. I guess this PR needs to be rebased on the master branch and that breaking change (use of |
|
@bikeshedder thanks for the offer of contribution, it is well received! That sounds like a good summary, there is also this #62 (comment) that needs to be resolved. |
|
This PR can be closed now that #84 is merged. |
PR draft as discussed on #59
There is a lot of duplicated code because
FluentLanguageLoaderandFluentMultiLanguageLoaderwork the same way for loading and bundling, with exception of retrieval of translated strings. Perhaps moving some of this functionality as free functions?Will work towards better docs of methods!