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

Inconsistent and misleading? behavior for FluentArgs::set #271

Merged
merged 2 commits into from Oct 27, 2022

Conversation

MathieuTricoire
Copy link
Contributor

Hi,

There is an inconsistent result when we set an argument that already exists, but I think the main issue is that when we set an argument that already exists it doesn't replace the argument in the underlying Vec but it is added.

There is an example that show the issue and my proposition to fix that.

use fluent_bundle::{FluentArgs, FluentBundle, FluentResource};

fn main() {
    let res = FluentResource::try_new(
        r#"
hello = Hello, { $user }.
hello-with-email-count = Hello, { $user }. You have { $emailCount } messages.
"#
        .to_string(),
    )
    .expect("Failed to parse FTL.");
    let mut bundle = FluentBundle::default();
    bundle.set_use_isolating(false);
    bundle.add_resource(res).expect("Failed to add a resource.");

    // simple hello
    let msg = bundle
        .get_message("hello")
        .expect("Failed to retrieve a message.");
    let value = msg.value().expect("Failed to retrieve a value.");

    let mut args = FluentArgs::new();
    args.set("user", "John");
    args.set("user", "Alice");
    let mut err = vec![];

    // Returns "Hello, John."
    assert_eq!(
        bundle.format_pattern(value, Some(&args), &mut err),
        "Hello, Alice."
    );

    // hello with email count
    let msg = bundle
        .get_message("hello-with-email-count")
        .expect("Failed to retrieve a message.");
    let value = msg.value().expect("Failed to retrieve a value.");

    let mut args = FluentArgs::new();
    args.set("user", "John");
    args.set("emailCount", 5);
    args.set("user", "Alice");
    args.set("emailCount", 7);
    let mut err = vec![];

    // Returns "Hello, Alice. You have 5 messages."
    assert_eq!(
        bundle.format_pattern(value, Some(&args), &mut err),
        "Hello, Alice. You have 7 messages."
    );
}

Copy link
Member

@gregtatum gregtatum left a comment

Choose a reason for hiding this comment

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

Thanks, that seems like a good change to me. I asked for one small change then I can merge this in.

fluent-bundle/src/args.rs Outdated Show resolved Hide resolved
@gregtatum gregtatum merged commit 681245b into projectfluent:master Oct 27, 2022
@MathieuTricoire MathieuTricoire deleted the fix-args-set branch October 27, 2022 19:33
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.

None yet

2 participants