Build translations from mdbook

[sakex]

Fixes #13.
Fixes #18.

[sakex]

We should bump the version as alpha in cargo.toml and publish the new version

[mgeisler]

This basically moves the for loop from publish.yml to the Rust code?

Later I guess this for-loop-renderer (🙂) will know to pass in i18n-releated information to the book — or maybe not since the other renderer will already be able to grab this from the book.toml file?

[sakex]

This basically moves the for loop from publish.yml to the Rust code?

Yes that's the only thing it does. The advantage is that we get a better developer experience for translators. They see all translations at the same time without having to rebuild multiple times with a different command. Live reload is also supported. It's a small win but I think it's worth it.

[mgeisler]

Just a note about Merge branch 'main' into translate-from-mdbook. Please rebase your commits on top of main instead of merging main into the branch. By rebasing, you create a clean and useful history — by merging, you end up with a mix of your changes and other changes and it's hard to read the history afterwards.

For this reason, I will want to squash merge the PR if it has merge commits.

[mgeisler]

From skimming Alternative Backends, I don't see any ordering between the renderers. Do we rely on the existing HTML output being present? If we do, we should probably not do this and generate it ourselves 🙂

In this renderer, we don't care about what runs first.

We don't care because we run the html renderer ourselves?

For the new template renderer, we will need html to be present.

Could you instead run the html renderer yourself? Output the files to your ctx.destination and the process each of them through the template engine.

The rendering order is based on the order in which they are defined in book.toml. I remember seeing it in the doc, but I can't find it anymore, reading the code, it is how it is handled.

Okay, thanks for checking! If it isn't documented, then we should not rely on it. If the order is important, could you then open an PR to the upstream repository to define the order in the documentation?

[sakex]

Just a note about Merge branch 'main' into translate-from-mdbook. Please rebase your commits on top of main instead of merging main into the branch. By rebasing, you create a clean and useful history — by merging, you end up with a mix of your changes and other changes and it's hard to read the history afterwards.

For this reason, I will want to squash merge the PR if it has merge commits.

Yes, I did that because it is more convenient. I only want one commit for this PR anyway, so squashing is expected

[sakex]

Hey @mgeisler . Is it Ok to merge now (with squash) so that I can use this as a base for the other renderer? After the renderer is done, I'll make a test book to test all the features

[mgeisler]

Hey @mgeisler . Is it Ok to merge now (with squash) so that I can use this as a base for the other renderer?

I think we need some documentation before we merge this. It's not clear to me what the expectations are for the user.

As an example, I believe you said that the html backend has to be enabled?

After the renderer is done, I'll make a test book to test all the features

Please add a test here already, otherwise it's hard to know what to expect. Also, update the first PR comment above to explain what this does (hint: reuse that text for your module docstring and for the README and USAGE).

[mgeisler]

From skimming Alternative Backends, I don't see any ordering between the renderers. Do we rely on the existing HTML output being present? If we do, we should probably not do this and generate it ourselves 🙂

In this renderer, we don't care about what runs first.

We don't care because we run the html renderer ourselves?

We still need to answer this essential question: how can we rely on ctx.destination + "/../html" existing when this backend is executed? I believe we still have that implicit expectation?

[sakex]

From skimming Alternative Backends, I don't see any ordering between the renderers. Do we rely on the existing HTML output being present? If we do, we should probably not do this and generate it ourselves 🙂

In this renderer, we don't care about what runs first.

We don't care because we run the html renderer ourselves?

We still need to answer this essential question: how can we rely on ctx.destination + "/../html" existing when this backend is executed? I believe we still have that implicit expectation?

No, it's not required anymore

[mgeisler]

Hey @sakex, I tried marking the things we need to figure out or explain before we merge this.

[mgeisler]

What is "selected language"?

[sakex]

The language set in the env variable MDBOOK_BOOK__LANGUAGE

[mgeisler]

Ah, okay... Setting MDBOOK_BOOK__LANGUAGE is just another way of setting the language field in book.toml. So it's not strictly "selected", it's just what the config happens to be when the mdbook-gettext preprocessor is invoked.

The current system has the elegant approach that you can set language to what you want and it will then translate the text on the fly to this language — because it's a preprocessor which is independent of the renderer!

The new renderer is a bit different: it takes a list of languages from book.toml and renders the book for each of them. If the user only want to render a single language, the simplest way would be for the user to remove the other languages from the list in book.toml!

The users could do this on the command line with a suitable MDBOOK_OUTPUT__I18n__LANGUAGES setting (I think). This would be better since it uses less infrastructure: all you need to know is that the output.i18n.languages key controls the list of languages to generate for. If it has 1 item, then you get a single language, if it has $n$ items, then you get $n$ languages.

[mgeisler]

The conversion is documented here: https://rust-lang.github.io/mdBook/format/configuration/environment-variables.html.

[sakex]

The reason for this variable is that if you do MDBOOK_BOOK__LANGUAGE=fr mdbook build with translate_all_languages to false, it will just translate fr otherwise, it will translate all languages. We can skip that, it's just for convenience.

[mgeisler]

The reason for this variable is that if you do MDBOOK_BOOK__LANGUAGE=fr mdbook build with translate_all_languages to false, it will just translate fr otherwise, it will translate all languages. We can skip that, it's just for convenience.

Can we unify this behavior? We stop setting MDBOOK_BOOK__LANGUAGE and instead drive the behavior of the backend using solely a new output.i18n.languages field. It should behave like this:

• The book.toml file checked in has all languages for the book.

• To build fewer languages, people can remove the unwanted languages from book.toml. Alternativley, they can use the environment variable support and run

  MDBOOK_OUTPUT__I18N__LANGUAGES='["fr"]' mdbook build

  to restrict the languages to just French.

Why have a list in book.toml instead of just using po/*.po? Because there could be extra meta data compared to what we find in the PO files. I'm thinking of things like english_name = "French", name = "Français" and maybe even available_in_language_picker = true.

Does that sounds like a good idea? So instead of a "all languages or a single language" based on MDBOOK_BOOK__LANGUAGE (which doesn't make a ton of sense for a multi-language book), we have a more flexible system that people can override on the command line.

[sakex]

I like the idea, it would require refactoring a bit.

In the current state, it works like this:

mdbook build runs normally on whatever language is set in the environment or manually in the book, then we execute a variable number of backends before reaching this one. From this backend, we loop over the languages defined in book.toml and do mdbook build for each language.

What you're suggesting would mean intercepting everything right at the beginning, so it would need to be a preprocessor that would need to run first and then orchestrate the language directives. It works too but would require some refactoring.

[mgeisler]

What you're suggesting would mean intercepting everything right at the beginning,

I'm not actually suggesting that — I was only suggesting changing the logic for which subset of the languages to build.

[mgeisler]

I think you're saying here that you'll break the contract defined by mdbook.

I don't want the new renderer to touch files in the output directories of other renderers.

[sakex]

It's very convenient with mdbook serve to view the output like it will be on the final site. It doesn't need to be enabled otherwise and by default does nothing

[mgeisler]

I think you're saying here that you'll break the contract defined by mdbook.

I don't want the new renderer to touch files in the output directories of other renderers.

It's very convenient with mdbook serve to view the output like it will be on the final site. It doesn't need to be enabled otherwise and by default does nothing

I don't think you addressed my concern: if the code is writing outside of the output directly, then the code is doing something wrong.

I don't doubt that it is convenient: we should just write everything inside of the designated output directory.

Maybe you're not aware, but I often use mdbook build -d /tmp/foo and similar to generate output in non-standard directories. So this renderer must respect the output setting.

[sakex]

By default, we don't break the contract but if we enable this, we do. That's also what we're already doing in comprehensive rust, so I don't really see a problem as it is opt-in only.

[mgeisler]

That's also what we're already doing in comprehensive rust, so I don't really see a problem as it is opt-in only.

I hope our mdbook plugins are not breaking this?

Are you talking about the code in publish.yml, then that is quite different: the code there relies on the contract of mdbook which says that it will put things into directory so-and-so. After calling mdbook build, we are of course free to move things around and mess with the book/ directory — we own it then!

This is the main difference between implementing this functionality as a wrapper script (as suggested in #18!) and writing it as a mdbook plugin. As a plugin, you are subjected to the rules of the surrounding system.

It might be a subtle difference, but I believe it's an important one. By following the principle carefully on every level of the stack, we will be able to build bigger "stacks" of software which behaves in a predictable manner when seen as a whole.

[sakex]

The script does this:

for language in languages:
    cmd(f"MDBOOK_BOOK__LANGUAGE={language} mdbook build . -d i18n-helpers/{language}")
    for renderer_to_move in move_translations_directories:
        cmd(f"mv i18n-helpers/{language}/{renderer_to_move} {renderer_to_move}/{language}")

Note that the move happens after the mdbook build command ran. Which respects this statement:

After calling mdbook build, we are of course free to move things around and mess with the book/ directory — we own it then!

[mgeisler]

The script does this

Sorry, I don't think I know what script you're referring to here?

[sakex]

This renderer I mean

[simonsan]

I sometimes open this PR to look at the current state, and I wanted to let a compliment here for you both. I really like your interactions and styles of giving feedback. May this become a great feature :) Much of love! 👍🏽