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

Build process suggestions #8

Closed
ghost opened this issue Mar 24, 2019 · 7 comments
Closed

Build process suggestions #8

ghost opened this issue Mar 24, 2019 · 7 comments
Labels
enhancement New feature or request

Comments

@ghost
Copy link

ghost commented Mar 24, 2019

The current build process is a bit complicated/fragile for the following reasons:

  1. The fuzzy matches msgmerge creates are not reported during cargo build
  2. errors are out of context (e.g. Obscure error message when msgmerge is not available #7, or sometimes when it doesn't find the po/$DOMAIN/ folder)
  3. include_i18n! panics when translations/ doesn't exist, but this folder is only created by compile_i18n!, which is executed after
  4. the translations folder, and .pot files are compilation artifacts, but aren't in target/
  5. CARGO_TARGET_DIR is only set by the user, Cargo only reads it (Set a target for build scripts' products rust-lang/cargo#5457), (
    let out_dir = Path::new(&env::var("CARGO_TARGET_DIR").unwrap_or("target/debug".into()))
    ). It will later be available in build scripts (maybe), but it is absent in proc_macros
  6. cargo build has to be run twice to apply changes in .po files

I suggest that:

  1. init_i18n and compile_i18n should be normal functions, that are to be called in a build script.
    Output of build scripts is inhibited by Cargo, except when it's formatted as cargo:warning=..., then it shows as a compilation warning. Warnings can be used to show msgfmt --statistics output (especially to see if there are new fuzzy translations).
  2. translations/ should be target/translations/
  3. target/debug/gettext_macros/$DOMAIN should be inside target/translations, because it's common to debug and release builds

So.. an example of the new process would be:

// src/main.rs
extern crate gettext_macros;
extern crate gettext_utils;
use gettext_macros::{i18n, include_i18n};

fn main() {
    let catalog = &include_i18n!()[0].1;
    let s = i18n!(catalog, "Translated string");
    println!("{}", s);
}
// build.rs
extern crate gettext_utils;

fn main() {
    gettext_utils::compile_i18n("domain", &["en", "fr", "de"]);
}

Because build script are called before the actual compilation starts, the first time compile_i18n is called, it would only do what init_i18n! does:

  • create the file with the domain and the list of languages
  • create empty .pot/.mo files for the new languages (though don't know which one it should create? cf. Make it possible to choose what is compiled #4, include_i18n! reads from the .mo, but init_i18n! creates a .pot)
  • create the needed po/$DOMAIN/$LANGUAGES.po files with msginit
  • issue a warning to tell the user to run cargo build again

Then during the first compilation, i18n! macros would populate the .pot file, and include_i18n! would silently include the contents of the (empty) .mo file (instead of crashing).

During the next build, compile_i18n would do what's listed above, then call msgmerge with the populated .pot files, and msgfmt to create the .mo file. This would allow include_i18n! to include the new translations directly, instead of having to call cargo build twice

@ghost
Copy link
Author

ghost commented Mar 24, 2019

As you might guess, I'm willing to work on this. :)

But I'm not very knowledgeable about the way gettext works in general, especially the purpose of the different files (pot vs mo for example). Since you've already open this issue #4 maybe the whole process can be simplified

@elegaanz
Copy link
Member

Hello 🙂

4. the `translations` folder, and `.pot` files are compilation artifacts, but aren't in `target/`

The .pot file should be present next to the .po files, since many translation platforms requires it to generate new translations.

* issue a warning to tell the user to run `cargo build` again

I don't really like that, but I don't know if there is a better way…

Otherwise, what you propose sounds good to me.

But I'm not very knowledgeable about the way gettext works in general, especially the purpose of the different files (pot vs mo for example)

The idea is that you generate a template (.pot), that is used to create new translation files and update them (.po). These .po files will be edited by the translators. Then they will be compiled to .mo files that are basically a binary/compressed version of the .po, that will be used by the app.

If you have any other question, I will be glad to answer. 😊

@elegaanz elegaanz added the enhancement New feature or request label Mar 24, 2019
@ghost
Copy link
Author

ghost commented Mar 24, 2019

* issue a warning to tell the user to run `cargo build` again

I don't really like that, but I don't know if there is a better way…

Yes... and when actually writing the code, I found out that every time a new msgid is added with i18n!, cargo build needs to be rerun. It's a bit like the table of contents in latex.

On the other hand, if the .mo files are compiled after the build (like how it's currently done with compile_i18n!), then cargo build has to be run twice when a .po file is updated.

@ghost
Copy link
Author

ghost commented Mar 30, 2019

Do translation platforms need the .mo files?

@elegaanz
Copy link
Member

No, only the app needs them.

@ghost
Copy link
Author

ghost commented Mar 30, 2019

Then PR #9 is ready :)

@elegaanz
Copy link
Member

Ok, thank you. I will try to review it today or tomorrow. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant