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

tox translation build: add tests for lunr.js language support #2564

Closed
wants to merge 4 commits into from

Conversation

ultrabug
Copy link
Member

@ultrabug ultrabug commented Sep 7, 2021

this should avoid forgetting that a locale added to mkdocs theme
translations should be supported by lunr.js to work

this should avoid forgetting that a locale added to mkdocs theme
translations should be supported by lunr.js to work
@ultrabug
Copy link
Member Author

ultrabug commented Sep 7, 2021

@oprypin I intend to revert #2497 and merge this in 2 days

@ultrabug
Copy link
Member Author

ultrabug commented Sep 7, 2021

Unless we want to disable the search plugin for missing locales with a WARNING log instead of crashing like mad?

@oprypin
Copy link
Contributor

oprypin commented Sep 7, 2021

@ultrabug Yes, I definitely don't think search plugin support should affect availability of languages. I was actually thinking perhaps a fallback language would be used in that case. Or whatever mkdocs-material does.

@ultrabug
Copy link
Member Author

ultrabug commented Sep 8, 2021

mkdocs-material does crash if you select a language it does not support

  File "/home/alexys/venv/ultrabug.fr/lib/python3.9/site-packages/material/partials/language.html", line 4, in top-level template code
    {% import "partials/languages/" ~ config.theme.language ~ ".html" as lang %}
  File "/home/alexys/venv/ultrabug.fr/lib/python3.9/site-packages/jinja2/loaders.py", line 197, in get_source
    raise TemplateNotFound(template)
jinja2.exceptions.TemplateNotFound: partials/languages/pp.html

update tox lunr.js check to only warn and not fail the CI
@ultrabug
Copy link
Member Author

ultrabug commented Sep 8, 2021

With latest commit, the search plugin will only WARN on unsupported lang configuration:

WARNING  -  Ignoring search.lang "ezn" as it is not a supported language code.
WARNING  -  Ignoring search.lang "coco" as it is not a supported language code.

Removing the offending locales from the search.lang config is fine since this config option allows to be empty and falls back to english anyway.

I've made the tox CI to WARN as well but not fail the CI when a lunr locale is missing.

If you're okay with this, I'll update the tests accordingly

@oprypin
Copy link
Contributor

oprypin commented Sep 8, 2021

@ultrabug Hmm sorry I'm not quite following the whole scope of this. I have given an example of the ideal state I imagine this in: #2535 (comment)

Secondary concerns: surely we don't want a warning if MkDocs has a translation but the search plugin doesn't know the locale for it? In that situation I'd prefer choosing (centrally, within MkDocs) a fallback language (normally English) the search plugin.

The reason I don't want it as an actual warning is that then people can't use --strict mode. If MkDocs has the right translation, I think the message should be INFO level.

@ultrabug
Copy link
Member Author

ultrabug commented Oct 8, 2021

Moved to #2602

@ultrabug ultrabug closed this Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants