projectfluent / fluent-rs

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

Resolve more pattern types into &str references #305

Open JasperDeSutter opened 1 year ago

JasperDeSutter commented 1 year ago

The most important change is in fluent-bundle/src/resolver/pattern.rs, the rest is adding missing codepaths that were previously only done through WriteValue and are now also needed in ResolveValue.

There are more cases where we can resolve to a borrowed str than just the TextElement case. For example there might be a select that resolves into a single TextElement, or a TermReference which is a simple string. With these changes, ResolveValue methods will be called until a pattern with more than 1 element is found, which needs string concatenation and is handled by the WriteValue methods as before.

When resolving into a FluentValue::Error, the WriteValue is called to get "{error}" formatting instead of an empty string.

JasperDeSutter commented 1 year ago

Before these changes, Cow::Borrowed was returned 14 out of 174 times in fluent-bundle test cases. After the changes, I'm seeing 67 out of 174.

gregtatum commented 1 year ago

@JasperDeSutter I didn't get a chance to finish reviewing all of your PRs before the holidays. I'll be back in the new year.

gregtatum commented 1 year ago

I'm back from the holidays, but may still need a few days to get caught up for reviews.

alerque commented 2 months ago

Rebased to update commit messages, get current CI jobs to run, and run cargo clippy on each commit. Working on rebase to main now.

alerque commented 2 months ago

...and also fixed the rustfmt issue.