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

Add links between translations #35

Open
mgeisler opened this issue Feb 15, 2023 · 10 comments
Open

Add links between translations #35

mgeisler opened this issue Feb 15, 2023 · 10 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mgeisler
Copy link
Collaborator

mgeisler commented Feb 15, 2023

When outputting translations, we need to add links between the corresponding pages in the translations. This will take two forms:

  1. A language picker for users. This is done via Add language picker menu comprehensive-rust#411.
  2. Links for robots so that search engines can index the pages reliably.

The second part is done by adding

<link rel="alternate" hreflang="en" href="https://google.github.io/comprehensive-rust/" />
<link rel="alternate" hreflang="pt-BR" href="https://google.github.io/comprehensive-rust/pt-BR/" />

to the head element of every page. The alternatives will link each page with its translated siblings. See this documentation for details.

@mgeisler
Copy link
Collaborator Author

To do this, we'll likely have to make changes in mdbook. The problem is the template have access to a {{ path }} variable, but this contains the path to the Markdown file! We need to generate the link elements statically (without JavaScript), so we cannot massage this like I do in google/comprehensive-rust#411.

@mgeisler mgeisler added the good first issue Good for newcomers label Feb 16, 2023
@jooyunghan
Copy link

jooyunghan/comprehensive-rust@8a7331b is my WIP for this. Which depends on another WIP changes in mdbook.

@mgeisler
Copy link
Collaborator Author

Hi @jooyunghan, oh very cool!

Your upstream patch looks nice, though people might want to have more motivation (you could point them to our TRANSLATIONS.md) and perhaps they won't like the hard-coded use of subdirectories for the languages.

However, even if they don't want to use that directory structure, it should be easy for people to move the output around afterwards (I'm thinking of a setup where people want per-language xx.example.com subdomains.)

While the patch is pending upstream, could you perhaps implement this directly in our repository with a bit of post-processing in publish.yml? We could add a simple marker to our index.hbs and then just use sed to replace it with the necessary link elements.

@mgeisler mgeisler transferred this issue from google/comprehensive-rust Jun 22, 2023
@mgeisler
Copy link
Collaborator Author

One way of solving this would be to write a tool which can add the necessary HTML after the book has been rendered. The advantages of this would be

  • It would give us an extensible way to post-process translations without having to change the upstream mdbook every time.
  • It would give us an entry point for the user. I'm thinking that mdbook-i18n build would build all translations, mdbook-i18n serve would show them all in your browser, etc... This would give us fine-grained control over things. This would also be how we can implement Add support for only publishing a language if it more than NN% translated #16.

@sakex
Copy link
Collaborator

sakex commented Sep 4, 2023

The first thing we should do is add a way to embed a JSON config inside of the page. Then we'll be able to use the data from Javascript to add the links.

My initial idea was to convert book.toml to a JSON and add that JSON as a script tag inside of the HTML rendered template. This would not be ideal because we would potentially write unwanted data to the HTML. The fix is to write a subset of book.toml.

The Config struct has a rest member that contains the remaining data inside of book.toml. We could add a [js_data] field to book.toml that we could read from our custom renderer to add extra data to the page.

@mgeisler
Copy link
Collaborator Author

mgeisler commented Sep 4, 2023

The first thing we should do is add a way to embed a JSON config inside of the page. Then we'll be able to use the data from Javascript to add the links.

Just a note: I would prefer to avoid adding more JavaScript to the pages. Main reason: it's annoying to test and debug. From what I can tell, JavaScript is supported for this kind of thing, but I would prefer a static page when we are capable of producing one.

Until then, a JavaScript solution could be okay.

@mgeisler
Copy link
Collaborator Author

mgeisler commented Sep 4, 2023

Oh, another point: the translation infrastructure is now used by a handful different projects, so we should take care to make it as easy as possible for them to use. The less custom code people have to add the better 🙂

@sakex
Copy link
Collaborator

sakex commented Sep 11, 2023

I'm starting to work on this. My first thought is that we could move the translation logic to the renderer so we wouldn't need to do this as part of the CI with a different command. WDYT?

@mgeisler
Copy link
Collaborator Author

My first thought is that we could move the translation logic to the renderer so we wouldn't need to do this as part of the CI with a different command. WDYT?

Using a preprocessor to do translations works really well and gives us a nice and important property: the search index is local to the current language (as one would expect). Similar for the print.html page that is generated. We can also have different assets for different languages.

The architecture of mdbook is that you work with one book — so I would not suggest changing this right now.

Also, as you know (#70), we don't yet have very good control over the renderer 😄 So I'm not sure how this would actually work?

@mgeisler
Copy link
Collaborator Author

mgeisler commented Nov 5, 2023

Hi @sakex, I was just playing with the new renderer from #80 and it works really well!

I tried solving this issue and it's just a few lines of template code now to remove the annoying JavaScript in head.hbs. I replaced it with:

{{{{raw}}}}

{% macro base_url(lang) %}
{%- set base_url = "https://google.github.io/comprehensive-rust" %}
{%- if lang != "en" %}
{%- set base_url = base_url ~ "/" ~ lang %}
{%- endif %}
{{- base_url }}
{%- endmacro input %}

{% macro canonical_path(path) %}
{%- if path is ending_with("index.html") %}
{%- set path = path | split(pat="/") | slice(end=-1) | join(sep="/") %}
{%- endif %}
{{- path }}
{%- endmacro canonical_path %}

{% set path = self::canonical_path(path=path) %}
{% set base_url = self::base_url(lang=ctx.config.book.language) %}
<link rel="canonical" href="{{ base_url }}/{{ path }}">

{% for lang in ctx.config.i18n.languages %}
{% set alt_base_url = self::base_url(lang=lang.code) %}
<link rel="alternate" hreflang="{{ lang.code }}" href="{{ alt_base_url }}/{{ path }}" />
{% endfor %}

{{{{/raw}}}}

For this to work, had to do two things. First, I adjusted the path to be relative to the html output directory:

diff --git a/mdbook-tera-backend/src/tera_renderer/renderer.rs b/mdbook-tera-backend/src/tera_renderer/renderer.rs
index 7439423..94f62c2 100644
--- a/mdbook-tera-backend/src/tera_renderer/renderer.rs
+++ b/mdbook-tera-backend/src/tera_renderer/renderer.rs
@@ -71,7 +71,8 @@ impl Renderer {
 
     /// Rendering logic for an individual file.
     fn render_file_content(&mut self, file_content: &str, path: &Path) -> Result<String> {
-        let tera_context = self.create_context(path)?;
+        let html_dir = self.ctx.destination.parent().unwrap().join("html");
+        let tera_context = self.create_context(path.strip_prefix(&html_dir)?)?;
 
         let rendered_file = self
             .tera_template

I think relative paths are the way to go when passing information to the templates.

Second, I added this little snippet to the book.toml file:

[i18n]
languages = [
  { code = "en", name = "English", en_name = "English" },
  { code = "da", name = "Dansk", en_name = "Danish" },
  { code = "pt-BR", name = "Português do Brasil", en_name = "Brazilian Portuguese" },
]

I figured the names could be reused for the building the language picker as well, so that's why I added them here. The order in the list is of course the order in which the link elements are rendered and it will also be the order for the language picker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants