Folder structure: get_file_from_path() returns wrong files and is_index() broken

unverbuggt commented 1 year ago

Hi,

I know it is clearly stated that the folder structure is not as mature as the suffix one, but I've been experimenting with the folder structure lately, as it would allow me to do two things:

use a second mkdocs.yml with a different default_language set
and only build the default language in the root folder (without redundant copy) So one configuration will build / (en) and /de (not /en) and the other would build / (de) and /en (not /de).

Furthermore, I also would like to host non localized images/videos just once as described here, but couldn't get it to not save redundant copies of said images/videos, yet.

I made a new branch for my configuration here. With the current mkdocs(1.5.2) and mkdocs-static-i18n (0.56) the project builds, but the menu structure is wrong in two ways:

One index file (colors/index.md) is returned as index.md by get_file_from_path(). This way we got a double entry in nav of the site and this breaks the navigation. I worked around with this patch, but I certainly don't fully understand what the code does or if this breaks something else:

--- C:/Users/Rene/AppData/Local/Programs/Python/Python38/Lib/site-packages/mkdocs_static_i18n/folder_structure.py.org   Sun Aug  6 17:05:26 2023
+++ C:/Users/Rene/AppData/Local/Programs/Python/Python38/Lib/site-packages/mkdocs_static_i18n/folder_structure.py   Sun Aug  6 17:03:55 2023
@@ -55,10 +55,7 @@
         expected_src_path,
         expected_src_path.relative_to(root_folder),
         Path(self.locale) / Path(expected_src_path),
-            Path(self.locale) / Path(expected_src_path.relative_to(root_folder)),
         Path(self.default_locale) / Path(expected_src_path),
-            Path(self.default_locale)
-            / Path(expected_src_path.relative_to(root_folder)),
     ]
     for src_path in filter(lambda s: Path(s) in expected_src_paths, self.src_paths):
         return self.src_paths.get(os.path.normpath(src_path))

The index files are not marked as such by the page.is_index() because the "file.name" is "index.md" (and not "index"). I intervened with the patch (copied from suffix structure):


--- C:/Users/Rene/AppData/Local/Programs/Python/Python38/Lib/site-packages/mkdocs_static_i18n/folder_structure.py.org   Sun Aug  6 17:05:26 2023
+++ C:/Users/Rene/AppData/Local/Programs/Python/Python38/Lib/site-packages/mkdocs_static_i18n/folder_structure.py   Sun Aug  6 17:03:55 2023
@@ -111,8 +108,8 @@
     self.locale_suffix = None
     self.root_folder = Path(file_from.src_path).parts[0]

the name
self.name = Path(self.initial_src_path).name
the name without any suffix
self.name = self._get_name() self.dest_name = self.name
```
 if self.root_folder not in self.all_languages:
```

Also, i18n_page_file_locale doesn't seem to work within folder structure.

Anyway, thank you for this great plugin, it is very useful.

ultrabug commented 1 year ago

Hello @unverbuggt ; thanks for your kind words, I'm happy to hear that the plugin is useful to you!

As I'm currently working on a major upgrade of the plugin I'm afraid I won't be able to tackle your problem on the old code. But I'll rather allow myself to gently ping you on the PR once I'm done with its folder structure support for testing unless you're against it.

Thanks

unverbuggt commented 1 year ago

Yes, just ping me. I'd be glad if I can help by testing.

ultrabug commented 1 year ago

Pinged you on the PR ! https://github.com/ultrabug/mkdocs-static-i18n/pull/216

unverbuggt commented 1 year ago

The "refactor/config" branch looks really promising:
I was able to successfully build with different default languages (in two different mkdocy.yml). All non-localized assets were only copied once. default language was only built in the root folder.

But it took me a while to adjust my theme to work with the "list of dicts" i18n_config.languages. Previous versions used a dict for the same variable name. If possible, please convert the i18n_config.languages list to a dict to remain backward compatible. You could serve the new config format under a different variable like i18n_static_config.languages or i18n_config.locales.

Having customizable nav per language is a neat feature, but it looks like it indexes all pages to the navigation even if they aren't in the configured nav. Also the overriding of the page title in the nav structure doesn't seem to work.

ultrabug commented 1 year ago

Thanks for your time and feedback @unverbuggt

please convert the i18n_config.languages list to a dict to remain backward compatible.

I wanted to do that but I would loose strict language code validation which comes with newer mkdocs version. I created a migration script to help people adapt their config to the new one as mentioned before.

Having customizable nav per language is a neat feature, but it looks like it indexes all pages to the navigation even if they aren't in the configured nav.

You are right, I just fixed nav being honored correctly. It now works as expected (I hope).

Also the overriding of the page title in the nav structure doesn't seem to work.

It is also working now with the previous fix.

unverbuggt commented 1 year ago

The menu is still not built correctly, I'm afraid.

In suffix structure nav reflects the markdown files in the docs directory. Translations share the same filename with a suffix added.

How should the nav yaml syntax be constructed for the folder structure?
I'm getting lots of WARNING - A relative path to 'index.md' is included in the 'nav' configuration, which is not found in the documentation files. and INFO - The following pages exist in the docs directory, but are not included in the "nav" configuration:

Do the navs in the languages replace the main nav in the mkdocs.yml? When I add the paths relative to docs dir (f.ex. en/index.md) the warnings disappear, but the menu shows None and the links are not working.

The main benefit of the folder structure (to me) is that it would allow to build with different default languages (from different mkdocs.yml with the same docs directory) and allows using only one copy of non-localized files (f.ex. videos which would take much space if copied redundantly). If it would also allow to have different sets of navigation (f.ex. some pages not available in all languages and no fallback to the default language needed/intended).
Could these goals also be achieved using the suffix structure?

ultrabug commented 1 year ago

How should the nav yaml syntax be constructed for the folder structure?

In principle, you should not have to modify the main nav configuration of your mkdocs.yml. Overriding the nav is possible per language but absolutely not required, even more to achieve your goal.

When using the suffix structure, one should never have to worry about adding suffixed paths to their main mkdocs.yml. And that's the same for folder structure users: paths should be expressed as if they were part of the locale they belong to (no fr/section/index.md but section/index.md)

...and allows using only one copy of non-localized files

That's also true for the suffix structure

...also allow to have different sets of navigation (f.ex. some pages not available in all languages and no fallback to the default language

This is valid for both suffix and folder structure. Just use the fallback_to_default: false config of the plugin!

While the suffix structure is more mature, my intention is to get both structures working the same way and producing the same output. They should share the same benefits, so the fact that it's not yet the case means that I still have work to do :)

ultrabug commented 1 year ago

@unverbuggt FYI I pushed some updated code to the folder structure, give it another try please!

unverbuggt commented 1 year ago

Thanks for the continued effort, just wanted to make sure, that I've configured it correctly.

The warnings about pages missing from nav are gone now.

my config is here. the nav part:

nav:
    - 'mkdocs.md'
    - 'index.md'
    - Markdown: 'markdown.md'
    - Colors:
      - 'colors/index.md'
      - 'colors/primary.md'
      - Secondary colors:
        - 'colors/secondary/index.md'
        - 'colors/secondary/mono.md'
        - 'colors/secondary/analogous.md'
        - 'colors/secondary/analogous2.md'
        - 'colors/secondary/complementary.md'
        - 'colors/secondary/complementary2.md'
        - 'colors/secondary/triadic.md'
        - 'colors/secondary/triadic2.md'
        - 'colors/secondary/compound.md'
        - 'colors/secondary/compound2.md'
    - Extensions:
        - 'extensions/static-i18n.md'
        - 'extensions/encryptcontent.md'
        - 'extensions/webfonts.md'
        - Javascript:
          - 'extensions/hljs.md'
          - 'extensions/tablesort.md'

is translated to this in the template:

Page(title='color-theme plugin', url='/mkdocs-risonia-theme/colors/')
Page(title='Primary colors', url='/mkdocs-risonia-theme/colors/primary/')
Page(title='static i18n plugin', url='/mkdocs-risonia-theme/extensions/static-i18n/')
Page(title='Encrypt content', url='/mkdocs-risonia-theme/extensions/encryptcontent/')
Page(title='Webfonts', url='/mkdocs-risonia-theme/extensions/webfonts/')
Section(title='Javascript')
    Page(title='highlight.js', url='/mkdocs-risonia-theme/extensions/hljs/')
    Page(title='tablesort', url='/mkdocs-risonia-theme/extensions/tablesort/')
Section(title='Secondary colors')
    Page(title='Secondary colors', url='/mkdocs-risonia-theme/colors/secondary/')
    Page(title='Monochomatic', url='/mkdocs-risonia-theme/colors/secondary/mono/')
    Page(title='Analogous', url='/mkdocs-risonia-theme/colors/secondary/analogous/')
    Page(title='Analogous 2', url='/mkdocs-risonia-theme/colors/secondary/analogous2/')
    Page(title='Complementary', url='/mkdocs-risonia-theme/colors/secondary/complementary/')
    Page(title='Complementary 2', url='/mkdocs-risonia-theme/colors/secondary/complementary2/')
    Page(title='Triadic', url='/mkdocs-risonia-theme/colors/secondary/triadic/')
    Page(title='Triadic 2', url='/mkdocs-risonia-theme/colors/secondary/triadic2/')
    Page(title='Compound', url='/mkdocs-risonia-theme/colors/secondary/compound/')
    Page(title='Compound 2', url='/mkdocs-risonia-theme/colors/secondary/compound2/')

That's what I meant by the nav is not built correctly (title override not respected and the Extension section moved to the root section.

But I've managed to find a workaround in jinja2 that works with the old and new language structure:

First find locale and name of languages plus find the default one

{%- if i18n_config %}
  {%- set i18n_theme = namespace(default="", languages=dict()) %}
  {%- if 'default_language' in i18n_config %}
    {%- set i18n_theme.default = i18n_config.default_language %}
    {%- for lang, display in config.plugins.i18n.config.languages.items() %}
      {%- set _ = i18n_theme.languages.update({lang: display.name }) %}
    {%- endfor %}
  {% else %}
    {%- for lang in i18n_config.languages %}
      {%- if lang.default %}
        {%- set i18n_theme.default = lang.locale %}
      {%- endif %}
      {%- set _ = i18n_theme.languages.update({lang.locale: lang.name }) %}
    {%- endfor %}
  {%- endif %}
{%- endif %}

this example is for the meta tag (alternate pages)

<head>
...

  {%- if i18n_config %}
    {%- for lang, name in i18n_theme.languages.items() %}
      {%- if lang != i18n_page_locale %}
        {%- if lang == i18n_theme.default %}
  <!--link from other language to default one -->
  <link rel="alternate" hreflang="x-default" href="{{ page.canonical_url | replace('/' + i18n_page_locale + '/', '/') }}">
        {%- elif i18n_page_locale == i18n_theme.default %}
  <!--link from default language to other one -->
  <link rel="alternate" hreflang="{{ lang }}" href="{{ page.canonical_url | replace(page.url, '') + lang + '/' + page.url | replace(i18n_page_locale + '/', '') }}">
        {%- else %}
  <!--link from language to other one -->
  <link rel="alternate" hreflang="{{ lang }}" href="{{ page.canonical_url | replace('/' + i18n_page_locale + '/', '/' + lang + '/') }}">
        {%- endif %}
      {%- endif %}
    {%- endfor %}
  {%- endif %}

...
</head>

ultrabug commented 1 year ago

@unverbuggt thanks, is your branch up to date? Once I'm done with updating tests to validate the folder structure I'll switch to testing your repository directly myself!

Writing folder tests allowed me to find some other mismatches which I'm in the process of fixing. Tests will ensure that both suffix and folder structure produce the same results!

I'll keep you posted

unverbuggt commented 1 year ago

yes, the "i18n_folder_structure" is up to date. I also tested the suffix structure in the "main" branch here which works fine so far.

ultrabug commented 12 months ago

Hi @unverbuggt I hope you're doing good!

FYI I just pushed the updated folder code which has now the same logic as the suffix one. That's confirmed and ensured by tests, which I'm happy about!

I'll try to make time to test with your repository but in case you can try again, please comment

unverbuggt commented 12 months ago

Hi,

now the menu builds correctly in forlder structure. Nice! But when the site is built, the non-localized images/videos are put in the root and in the locale directory. They should only be in the root directory, or do I need to move them to the "assets" directory specifically?

I got one small other issue left, and I currently cannot spot where exactly something goes wrong: i18n_page_locale doesn't reach the page's context in on_page_context called from another plugin (encryptcontent) that is ordered after i18n_static for some reason. However, it certainly reaches the theme template, and it is written to context and returned in your plugin's on_page_context. In fact i18n_config and i18n_page_locale are missing from the context for the other plugins. So it seems like, that it is deleted from the context somewhere, but I can't find where…

ultrabug commented 12 months ago

But when the site is built, the non-localized images/videos are put in the root and in the locale directory. They should only be in the root directory, or do I need to move them to the "assets" directory specifically?

Good catch indeed, I've pushed a fix to that thank you!

I'll check about the other part of your comment ASAP

ultrabug commented 12 months ago

i18n_page_locale doesn't reach the page's context in on_page_context called from another plugin (encryptcontent) that is ordered after i18n_static for some reason

Indeed I use the new plugin priority feature which allows plugins to ask to be run with a specific priority

I initially set on_page_context and on_template_context to be run LAST which is why you're experiencing this. I fixed that now!

unverbuggt commented 12 months ago

Thank you very much, this solves all issues for me.

ultrabug commented 12 months ago

Thank you so much too for your continued tests and feedback!

ultrabug / mkdocs-static-i18n

Folder structure: get_file_from_path() returns wrong files and is_index() broken #237

the name

the name without any suffix