Parameter order is incorrect and appears to be random.

[dessalines]

I've found an issue when using multiple parameters, the ordering is incorrect and random.

Rust:

&lang.notification_post_reply_body(&person.name, &comment.content, &inbox_link)

JSON:

"notification_post_reply_body": "<h1>Post reply</h1><br><div>{username} - {comment_text}</div><br><a href=\"{inbox_link}\">inbox</ a>",

Yet when I do a hover / check the order, it shows:

pub fn notification_post_reply_body(&self, comment_text: impl ::std::fmt::Display, inbox_link: impl ::std::fmt::Display, username: impl ::std::fmt::Display) -> ::std::string::String

Somehow the order of name, comment content, inbox link, got changed to comment content, inbox link, username during the rosetta build process.

[kevincox]

It looks alphabetical? Is that intentional?

If it is intentional it would probably be better to make the arguments more like keyword arguments because this is a footgun.

[dessalines]

Yep, seems alphabetical, rather than ordered by position.

[baptiste0928]

It looks alphabetical? Is that intentional?

Yes, parameters are ordered in alphabetical order to avoid breaking changes in the generated code if parameters are reordered in translations. Since all parameters have the same type (impl Display), reordering will not produce any compiler error, and this be unnoticeable.

If you are using an extension like rust-analyzer on VS Code, parameters are shown when typing the function, so there shouldn't be any confusion. However, it's obviously not the case with all editors, I understand that it can be confusing.

Some ideas to solve the problem:

• Add a configuration option in rosetta-build to select which order to use (position or alphabetical)
• Function-like macro to set parameters with a key-value style (t!(lang, "key", foo = bar) - #7)

[kevincox]

A simpler solution than a macro could just be generating a struct as "keyword arguments". Then you can do one of the following. (I like the second because it can implement Display to avoid an allocation and could potentially support other methods later).

notification_post_reply_body(NotificationPostReplyBody{
  comment_text: &comment.content,
  inbox_link: &inbox_link,
  username: &person.name
})

NotificationPostReplyBody{
  comment_text: &comment.content,
  inbox_link: &inbox_link,
  username: &person.name
}.to_string()

This way it is really difficult to make this mistake, even if you are looking at simple diffs for code review.

[dessalines]

Add a configuration option in rosetta-build to select which order to use (position or alphabetical)

I def like that option the best, with maybe the default being positional.

Also it'd be good to note that alphabetical ordering is how its currently done in the docs. Took me a while to find this, because its not how I've seen any other i18n do it, and there was nothing in the docs about alphabetical ordering.

[baptiste0928]

A simpler solution than a macro could just be generating a struct as "keyword arguments".

The first option repeat the key name twice, which can be quite long. For the second one, it will be necessary to add a lang field to each struct, which can be quite repetitive. However, I think it's better that macros to specify key names

What about something like typed-builder, with a generated builder type with generics to ensure all parameters are provided at compile time and that implement Display.

lang
    .notification_post_reply_body()
    .comment_text(&comment.content)
    .inbox_link(&inbox_link)
    .username(&person.name)
    .to_string()  // Implements `Display` if all parameters are provided

It is very similar to struct for keys with multiple parameters, but does not require any additional import. For keys with fewer parameters, it is more readable, in my opinion.

lang.notification_mention_title().username(&person.name)

// vs

lang.notification_mention_title(NotificationMentionTitle{
  username: &person.name
})

// vs

NotificationMentionTitle {
  lang,
  username: &person.name
}

It also combine advantages of your both proposals: the builder implement Display and it is a method of the Lang type.

[dessalines]

I definitely like the explicit typed builder approach best.

[baptiste0928]

I will start working on a 0.2 release of rosetta next week, with the builders for keys with arguments :+1: