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

Support theme localization (rework of #1778). #2299

Merged
merged 85 commits into from May 6, 2021

Conversation

ultrabug
Copy link
Member

This branch is an attempt to renew the efforts put by @anlau
in #1778 in providing builtin theme localization support.

mkdocs and readthedocs themes will support the 'locale' parameter
which will use localized messages.po files to translate the theme
templates.

Contributors will thus be free to propose new messages.po for
their own language.

This PR proposes the 'en' (default) and 'fr' locales which should
be compiled using : pybabel compile -d locales from the theme
directory before they can be used.

This code has also been tested and is compliant with the
mkdocs-static-i18n plugin. The two of them combined provide a
fully featured way to internationalize/localize your documentation.

This branch is an attempt to renew the efforts put by @anlau
in mkdocs#1778 in providing builtin theme localization support.

mkdocs and readthedocs themes will support the 'locale' parameter
which will use localized messages.po files to translate the theme
templates.

Contributors will thus be free to propose new messages.po for
their own language.

This PR proposes the 'en' (default) and 'fr' locales which should
be compiled using : `pybabel compile -d locales` from the theme
directory before they can be used.

This code has also been tested and is compliant with the
`mkdocs-static-i18n` plugin. The two of them combined provide a
fully featured way to internationalize/localize your documentation.
Copy link
Member

@waylan waylan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far. Of course, except for resolving the merge conflict, it appears that we still need to address all of the items listed in this comment.

mkdocs/localization.py Outdated Show resolved Hide resolved
@ultrabug
Copy link
Member Author

  • Update the readthedocs theme to support translations (as has already been done with the mkdocs theme).

@ultrabug
Copy link
Member Author

  • List babel in the requirements files (in addition to setup.py).

@ultrabug
Copy link
Member Author

@waylan looking at babel compilation automation I see that there's both a github deploy-release and setup.py publish.

Are they both used? should I work on adding babel compilation to both?

Also, requirements/packaging.txt does not seem to be referenced anywhere, is that intended or should I also add babel in there?

@ultrabug
Copy link
Member Author

Friendly ping on my questions above @waylan ; I have the modifications ready but don't want to commit useless stuff. Your input is appreciated. Thanks!

@waylan
Copy link
Member

waylan commented Feb 13, 2021

Hmm, I typed up a response the other day and now it's gone. Must be I never submitted?? Sorry about that. Let's try this again...

We use the GitHub workflow defined at .github/workflows/deploy-release.yml to automate MkDocs releases. Each time a new tag is created, that workflow automatically builds a MkDocs Python package and uploads it to PyPI.

requirements/packaging.txt is only used by the MkDocs release manager (me) as a backup to do a manual release if the automated release fails (which hasn't happened since we switched to using GitHub's system--but it should still work just in case).

In both cases, all of the dependencies in setup.py are installed before the build. Therefore we only need to be concerned with dependencies specifically required for doing a release which are not needed by the user (and therefore not listed in setup.py). The same changes would need to be made to both to ensure that the manual release method is not different from the automated method.

Note that we didn't make use of requirements/packaging.txt in .github/workflows/deploy-release.yml because we didn't need twine (the only common dependency was 'wheel' so we jus listed it explicitly). Instead we make use of the pypa/gh-action-pypi-publish action. I don't recall if it relies on setup.py publish or twine or something else. Whenever I have done a manual release I have called twine directly, which explains why it is listed in requirements/packaging.txt. However, in both cases, the "build" command is the same: python setup.py bdist_wheel sdist --formats gztar. Of course that just calls setup.py build under the hood before wrapping it up into wheel and gztar files. So, in the end, you should really only need to be concerned that setup.py build includes everything and that any additional dependencies which are only needed for the build process are listed both in requirements/packaging.txt and in the Install dependencies step of .github/workflows/deploy-release.yml.

@waylan
Copy link
Member

waylan commented Feb 13, 2021

For completeness, when doing a manual release I would issue the following commands after ensuring that the git working is tree up-to-date and creating the git tag:

rm -rf build
rm -rf dist
pip install --update -r requirements/packaging.txt
pip install --update -e .
python setup.py bdist_wheel sdist --formats gztar
twine upload dist/*

@ultrabug
Copy link
Member Author

Thanks @waylan ; I think this commit should do: cc5bd35

  • Work out the development workflow and possibly develop scripts/sub-commands to automate it.

@waylan
Copy link
Member

waylan commented Feb 15, 2021

You are making some good progress. I still need to do a thorough review, but a brief look through everything looks good. There are no glaring concerns. To recap, it looks like the following is completed:

  • Resolve the existing merge conflict.
  • Update the readthedocs theme to support translations (as has already been done with the mkdocs theme).
  • List babel in the requirements files (in addition to setup.py).
  • Document everything. We need to document things for users, theme devs, and translators; presumably separately.
  • Work out the development workflow and possibly develop scripts/sub-commands to automate it.

It appears that the last point is only partially completed. We still need the following:

  1. Create PO templates for each theme. These files would leave the translations blank so that translators could make a copy and start filling in the translations. Do we autogenerate these or just make hardcopies?
  2. We also need a workflow for us developers to update the MO files outside of a package build. Specifically, those of us who install mkdocs using develop mode (pip install -e). Yes, we could run the pybabel command, but I'm not going to remember the proper options every time. A simple wrapper which accepts the theme name and then runs the command with the options calculated would be helpful. Note that this tool needs to be accessible by translators (to test their translations) and possibly by third party theme devs. We won't require third party themes to use the same tools, but we should provide them as we need them for ourselves anyway. It might make sense for this to be a mkdocs subcommand. I'm not sure though. I haven't really thought it through all the way.

@ultrabug
Copy link
Member Author

  • Document everything. We need to document things for users, theme devs, and translators; presumably separately.

As for the rest of your points, I think this is addressed in the theme translation contributing guide so there's no blocker imho.

I'll let you check this out and hope you like it @waylan

- release notes
- styling your docs theme options + section
- contribution guide
Copy link
Member

@waylan waylan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ultrabug great job moving this forward.

I'm wondering if we should hardcode theme.locale as a setting in mkdocs.theme.Theme like we do static_templates. That way we can be sure it always exists and has a default. Or maybe it makes more sense to leave it to be defined by the theme. That way a theme which doesn't provide any translations (MO) files doesn't need to bother with it.

A few other concerns are addressed below.

docs/about/themes-translation.md Outdated Show resolved Hide resolved
docs/user-guide/styling-your-docs.md Outdated Show resolved Hide resolved
mkdocs/config/config_options.py Show resolved Hide resolved
@ultrabug
Copy link
Member Author

ultrabug commented May 4, 2021

Ok I'll make time to try out and document what's left of this tomorrow I promise.

Thanks again for all your hard work. It has been a windy road

I was prepared for it and furthermore I was not alone on that road thanks to you, oprypin and Martin ;)

The end is near and I hope the community will appreciate our efforts 👍

EDIT:

However, that is not related to this issue so feel free to open a separate PR to address that.

Okay, we'll do that separately

@ultrabug
Copy link
Member Author

ultrabug commented May 5, 2021

Ok I tried this out and I just pushed:

  • rewording of scripts to commands when describing setup.py commands
  • adding of a simple custom theme translation workflow based on mkdocs-basic-theme (I guess I'll contribute something there too when we're done here)
  • adding a note on custom theme translation inheritance / overriding of the extended theme translations

Now I'll work on documenting messages.mo packaging. Since theme developers are free to do whatever they want, I feel like we should explain both options to either commit & push the messages.mo file or not.

@waylan
Copy link
Member

waylan commented May 5, 2021

I had some time and finished up the documentation. @ultrabug unless you had other concerns I think this is ready for a final review and merge.

Copy link
Member

@waylan waylan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just took a final read through the changes and only noted one minor issue as outlined below.

.github/workflows/deploy-release.yml Outdated Show resolved Hide resolved
@ultrabug
Copy link
Member Author

ultrabug commented May 5, 2021

bb5b251 is a fix that we did not catch since we changed the POT files headers; this broke new catalog initialization

  File "/home/alexys/venv/mkdocs/lib/python3.8/site-packages/babel/messages/catalog.py", line 47, in _parse_datetime_header
    tt = time.strptime(match.group('datetime'), '%Y-%m-%d %H:%M')
  File "/usr/lib/python3.8/_strptime.py", line 562, in _strptime_time
    tt = _strptime(data_string, format)[0]
  File "/usr/lib/python3.8/_strptime.py", line 349, in _strptime
    raise ValueError("time data %r does not match format %r" %
ValueError: time data 'YYYY-MM-DD HH:MM' does not match format '%Y-%m-%d %H:%M'

now works as expected

@ultrabug
Copy link
Member Author

ultrabug commented May 5, 2021

Tried the whole workflow again here to add a new locale translation, everything looks easy and good 👍

I'm excited to get this in for the community!

@waylan
Copy link
Member

waylan commented May 5, 2021

Great! I expect to revisit it tomorrow and, barring any new concerns, merge it then.

@waylan waylan merged commit e1b77ab into mkdocs:master May 6, 2021
@waylan
Copy link
Member

waylan commented May 6, 2021

This has been merged 🎆 🎉. Thanks to everyone who helped move this forward. A big thanks to @ultrabug who did most of the work and was very patient with my ever-changing demands. 💯 🥇 🥇 We also want to give a special thanks to @anlau who got the work started. 🥇

@waylan waylan mentioned this pull request May 6, 2021
@ultrabug
Copy link
Member Author

ultrabug commented May 6, 2021

OMG 🏅 !! Thanks a lot, I hope this work proves useful to lots of users 🎉

@d0ugal
Copy link
Member

d0ugal commented May 6, 2021

Amazing work! Good job everyone! ❤️

@waylan waylan mentioned this pull request Jun 1, 2021
@oprypin oprypin mentioned this pull request Jun 4, 2021
oprypin added a commit to oprypin/mkdocs that referenced this pull request Aug 7, 2022
This check has only caused a lot of spurious failures. I never check it. In my view this approach is still a non-starter, and my comments had predicted the problems quite spot-on: mkdocs#2299 (comment)
oprypin added a commit to oprypin/mkdocs that referenced this pull request Aug 7, 2022
This check has only caused a lot of spurious failures. I never check it. In my view this approach is still a non-starter, and my comments had predicted the problems quite spot-on: mkdocs#2299 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants