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

adding localisation logic #49

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jeremieb24
Copy link
Contributor

i added localisation logic in MHWM installer and gui
in the installer, i added a page to select the installation locale. it would be saved in the configuration file and i added args and UAC logic to keep the current flow.

in the gui i added a picklist in the settings to change the locale and save it to the config file.

To add a Localisation:

  • add a Locale enum value for the locale to add
  • add the locale to Locale::ALL constant for picklist to use it
  • add the matching arm in the fmt implementation for the display of that locale in picklist
  • for each message already matched in the locale.message function, add a matching arm for the newly added locale with the text to return
  • in main.rs from the installer package, add a matching arm for the locale where the app is relaunched due to UAC

you can define any text to be localisable by adding a value to the Locale::Message enum and adding a translation for each existing locale in the locale.message function.

to use those localisation, use locale.message(Locale::Message::MessageToDisplay)

the only thing left to do is find a way to localise text that use format!() they currently do not work since they are allocated at runtime...im open to suggestions at this point... im not good enough with rust to know the exact workaround...

once the logic is done and working, i can implement it for english locale first, and in future PR i will add french.
then we can make a list of all known message kinda like your translation google doc you have so its easier for people to produce translation

id love your feedback on the general idea/logic i implemented and if you see any concern or have questions.

@jeremieb24
Copy link
Contributor Author

jeremieb24 commented Sep 16, 2024

i might create an issue to link this to later

Copy link
Member

@fenhl fenhl left a comment

Choose a reason for hiding this comment

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

Localization is a great feature to have, so thank you for starting the work on this! Here's some comments on the current draft:

Page::SelectLocale { .. } => (
{
let mut col = Column::new();
col = col.push("Select language you wish to proceed with");
Copy link
Member

Choose a reason for hiding this comment

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

This text should probably be kept as simple as possible, so someone with minimal knowledge of English can understand it:

Suggested change
col = col.push("Select language you wish to proceed with");
col = col.push("Select language");

Comment on lines 17 to 20
pub const ALL: [Locale; 2] = [
Locale::EN,
Locale::FR,
];
Copy link
Member

Choose a reason for hiding this comment

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

Instead of hardcoding this list, I recommend deriving enum-iterator::Sequence on Locale. Usages of Locale::ALL can then be replaced with enum_iterator::all() (or all::<Locale>() if Rust can't infer the type).

Locale::FR,
];

pub fn message(&self, message: Message) -> &str {
Copy link
Member

Choose a reason for hiding this comment

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

To support formatted strings, a good tradeoff between convenience and performance is the Cow type:

Suggested change
pub fn message(&self, message: Message) -> &str {
pub fn message(&self, message: Message) -> Cow<'static, str> {

You can then use Cow::Borrowed("...") for string literals and Cow::Owned(format!("...")) for formatted strings.

@fenhl fenhl added type: enhancement New feature or request component: installer Related to the multiworld installer component: GUI Related to the multiworld app labels Sep 16, 2024
@fenhl
Copy link
Member

fenhl commented Sep 16, 2024

I don't have a lot of experience with localization, but I feel like having all the translations in one function might get out of hand quickly. Maybe having a translate! macro that can be searched for, and keeping the text where it's used, would work? I'll definitely have to do some experimentation here.

i might create an issue to link this to later

I don't think that's necessary; this PR thread can be used to discuss the feature.

@jeremieb24
Copy link
Contributor Author

I don't have a lot of experience with localization, but I feel like having all the translations in one function might get out of hand quickly. Maybe having a translate! macro that can be searched for, and keeping the text where it's used, would work? I'll definitely have to do some experimentation here.

i agree, ill look into it. this will be a good opportunity for me to learn more about macros.

@fenhl
Copy link
Member

fenhl commented Sep 16, 2024

Here's what I think usage of such a macro could look like:

translate!(self.locale; {
    FR if pj64_version.is_none() => Cow::Borrowed("Ouvrir Project64"),
    EN => Cow::Owned(format!(
        "Open Project64{version}",
        version = if let Some(pj64_version) = pj64_version {
            format!(" version {pj64_version}")
        } else {
            String::default()
        },
    )),
})

The idea is that a locale can have an if guard, and if the guard fails or if the locale is missing entirely, the macro falls back to using the English locale. This is important to make sure development of new features can happen without immediately requiring everything to be translated into every supported language. In the example above, the “open Project64” button has been changed to say “open Project64 version x.y.z” for some reason, but the French translation for that hasn't been written yet (and maybe there are other locales that are supported in general, but this text hasn't been localized for those yet). There are already some instances of this pattern (just without the macro) in the midos.house repo (example).

There could even be a command similar to cargo sqlx prepare which generates a report of which translations are missing or incomplete. This would probably have to be written as a procedural macro, which is not quite the easiest Rust feature.

@jeremieb24
Copy link
Contributor Author

jeremieb24 commented Sep 16, 2024

i don't know enough about macros yet, but for the language fall back cant thi be done using some sort of match all arm like so (i have not tested this):

pub fn message(&self, message: Message) -> Cow<'static, str> {
        match message {
            Message::OpenPj64Button(pj64_version) => { // used in gui
                match self {
                    Locale::FR if pj64_version.is_none() => Cow::Borrowed("Ouvrir Project64"),
                    _ => Cow::Owned(format!(
                        "Open Project64{version}",
                        version = if let Some(pj64_version) = pj64_version {
                            format!(" version {pj64_version}")
                        } else {
                            String::default()
                        },
                    )),
                }
            }
        }
    }

message() could be rename to formatToLocale() , lookupLocale()
the reason i would put all the translations in one file (or message could be separated by view/page) is because i thought it would become hard to read if the translation logic is made directly where we would normally write text.
So the view/ui doesn't need to know the text to write, it would just ask the locale/translator to give him the text to write.

not sure if that makes sense. i like to see the whole localization enum as an actual dictionary... so if you want to write the text for that button, you just ask the locale to give you the appropriate text according to the user language
the same way you would look up a word in a dictionary

this is just my take on the subject. i don't have much experience in localization either, and in Rust.
in both cases, this will bring changes to the workflow for displaying text to the user.

@fenhl
Copy link
Member

fenhl commented Sep 18, 2024

i don't know enough about macros yet, but for the language fall back cant thi be done using some sort of match all arm […]

Yes, this does work and should be perfectly fine as an initial implementation of the feature. I'm just thinking ahead on how best to make localizations maintainable in the future.

@jeremieb24
Copy link
Contributor Author

jeremieb24 commented Sep 18, 2024

fair enough, i'll do some research on the subject, see how its usually done. ill get back once i have some insight on this

I might do some research on xtr crate

@jeremieb24
Copy link
Contributor Author

if we want to use the a key lookup structure, there is rust-i18n that could work. were you basically initialize it with the folder to look for translation and a fallback language, then you define your key:value in that folder using toml/json/yaml files
example:

//app.yml
_version: 2
hello-name:
  en: Hello, %{name}
  fr: Bonjour, %{name}
non-translated-text: 
  en: not found for locale %{lang}, defaulting in en locale


// main.rs
use rust_i18n::t;

rust_i18n::i18n!("locales", fallback = "en");

fn main() {
    rust_i18n::set_locale("fr");
    let name = "MinouPitou"; 
    println!("{}",t!("hello-name", name => name));
    println!("{}",t!("non-translated-text", lang => "fr"))
}

there is a vs code extension that can help with management of those translation with various tool

and there is a cli tool that can generate todos with missing translations.

ill try research different approach than key:value solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: GUI Related to the multiworld app component: installer Related to the multiworld installer type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants