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

Added localization language support #405

Merged
merged 44 commits into from
Jul 25, 2019
Merged

Conversation

macagua
Copy link
Contributor

@macagua macagua commented Apr 1, 2017

  • Added i18n support using Babel
  • Added Spanish translation
  • Updated .gitignore file

@Blendify
Copy link
Member

Blendify commented Apr 1, 2017

Awesome IMO we should hold off on 0.2.5 until this gets merged.

@Blendify
Copy link
Member

Blendify commented Apr 1, 2017

Once this gets merged I can talk to some of our translators and get the following translations:

  • De
  • Fr
  • It
  • Ja
  • Pt
  • Ru
  • Zh (Simplified)
  • Zh (Traditional)

Copy link
Contributor

@mitya57 mitya57 left a comment

Choose a reason for hiding this comment

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

  • Please do not include the binary .mo files in the repository. Storing the binary files in Git is a bad idea, and this file is (re-)created by Sphinx during build anyway.
  • The .gitignore changes seem unrelated.

Once this is merged, I will be happy to provide a Russian translation. I have it ready already :)

@macagua
Copy link
Contributor Author

macagua commented Apr 2, 2017

@mitya57 you are right about .gitignore file I revert this commit :-)

@mitya57
Copy link
Contributor

mitya57 commented Apr 2, 2017

It was better to rebase without that commit, rather than reverting.

But I am not the maintainer so I have no voice here anyway :)

@macagua
Copy link
Contributor Author

macagua commented Apr 3, 2017

@mitya57 please check out these improvements!

Copy link
Contributor

@mitya57 mitya57 left a comment

Choose a reason for hiding this comment

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

Still adding the binary .mo file. Also, left an inline comment for setup.py changes.

setup.py Outdated
'Sphinx>=1.4.1',
],
cmdclass = {
'compile_catalog': babel.compile_catalog,
Copy link
Contributor

Choose a reason for hiding this comment

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

If babel was not imported, this would be an undefined variable access. You can see this failure on Travis.

In my opinion setup.py modifications are not needed at all. This project is using setuptools, and babel is installing an entry point for setuptools, so these commands are available automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitya57 it is the problem :(

then remove the Babel Distutils/setuptools integration? I had to install Babel via pip install so I could extract the strings to translate and compile the mo file.

Copy link
Contributor

Choose a reason for hiding this comment

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

When babel is installed, its setup.py commands should work automatically, without your changes to setup.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitya57 An good alternative will be create a requeriments-devs.txt file with the Jinja2 and Babel dependencies or create a TRANSLATE file with this instructions.

@macagua
Copy link
Contributor Author

macagua commented Apr 3, 2017

@mitya57 Removed sphinx.mo file 8f21fd0

… TRANSLATE file for more details about translate the package
@macagua
Copy link
Contributor Author

macagua commented Apr 3, 2017

@mitya57 done 👍

@macagua
Copy link
Contributor Author

macagua commented Apr 7, 2017

@mitya57 any update about this?

@mitya57
Copy link
Contributor

mitya57 commented Apr 7, 2017

@macagua As I said, I am not the maintainer here, so someone else should merge this.

It looks good to me, though I would squash all commits into a single one to remove the useless changes (this can be done when merging though).

@Blendify
Copy link
Member

Blendify commented Apr 7, 2017

LGTM

@macagua
Copy link
Contributor Author

macagua commented Apr 8, 2017

@mitya57 thanks you!!!
Maybe @agjohnson or @ericholscher can check and merge this improvement?

@Blendify
Copy link
Member

Blendify commented Apr 8, 2017 via email

Copy link
Collaborator

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

I noted some documentation changes, but otherwise I think this makes sense.

Our current workflow on RTD is to use Transifex. If there is anything here that competes with that, I'd rather update our process here to match.

README.rst Outdated
============

You can help to translate the Read the Docs Sphinx Theme,
please check out the ``TRANSLATE.rst`` file for more details.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm -1 on littering the repo with these files, this should be in our documentation instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agjohnson inside a docs directory with a Sphinx documentation format?

Copy link
Member

Choose a reason for hiding this comment

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

I think inside the demo docs. Are general idea is to make them include more information about the theme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Blendify inside demo_docs folder?

TRANSLATE.rst Outdated
Languages availables:

- Spanish contributed by Leonardo J. Caballero G.
- Your Language by YOUR NAME.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file should be moved into our docs. Also, I'm very happy to apply attribution, but putting author and translation status information in the docs is guaranteed to go stale. I'd rather avoid this and keep authors in another location.

Copy link
Member

Choose a reason for hiding this comment

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

I agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agjohnson @Blendify when you said "This file should be moved into our docs" are you refernce into the docs readthedocs.org? Or what kind of location are you preference? or what other location are you preference?

TRANSLATE.rst Outdated
Installation
============

For translate the Read the Docs Sphinx Theme you need install the following packages:
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • For translate -> For translating
  • you need install -> you will need to install

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agjohnson done with commit 15d2198

TRANSLATE.rst Outdated

.. code:: bash

$ pip install babel Jinja2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Jinja2 is a sphinx requirement and should already be present as part of development set up for this package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agjohnson I agree with you, but it is not clear when downloading this package so at least I indicate that dependencies minimally require to generate the gettext files with the Babel library from this package.

Remember that when you install this package using the command python setup.py install does not install any dependencies, ie. does not install Sphinx package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agjohnson Any news on this comment?

TRANSLATE.rst Outdated
--------------

It is available the option to init catalog.
More options please, check out http://babel.pocoo.org/en/latest/setup.html#update-catalog
Copy link
Collaborator

Choose a reason for hiding this comment

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

This disutils section doesn't seem to clarify anything. It can either be dropped completely, or updated to be a list of explicit steps an author will need to take to create a translation.

@agjohnson agjohnson added PR: work in progress Pull request is not ready for full review and removed PR: ready for review labels Apr 13, 2017
@readthedocs readthedocs deleted a comment Jun 18, 2017
@readthedocs readthedocs deleted a comment Jun 18, 2017
@readthedocs readthedocs deleted a comment Jun 18, 2017
@daonkim
Copy link

daonkim commented Nov 19, 2018

Wow

@macagua
Copy link
Contributor Author

macagua commented Jan 22, 2019

@Blendify How can help to merge this improvements?

@derdennis
Copy link

Hello all,

I authored #587 almost a year ago and wanted to ask, if there is anything I can do to help with getting the proper internationalisation of this theme along? My impression is, that almost everything depends on this #405?

@joerg-vandervlies
Copy link

Hello everybody,

is anybody working on this merge request? I'd really appreciate to get this functionality.

kind regards
Jörg

@derdennis
Copy link

Hey Jörg,

is anybody working on this merge request? I'd really appreciate to get this functionality.

I don't know anything about the current work on this, but it is possible to use this functionality right now.

The folder locales in the source directory of your Sphinx project gets automagically searched for translations. If it is not there just create it. The needed structure looks like this:

source/locales/$LANG/LC_MESSAGES/sphinx.po

Where the $LANG part is a language identifier like it, ru or de.

If you want to use other folders than locales you can set the locale_dirs value in your conf.py as seen here.

I used this technique, copied in large parts from this comment, successfully with my German sphinx.po from here. The folder in locales is therefore named de.

Works like a charm.

@joerg-vandervlies
Copy link

Hi derdennis,

[...]
Works like a charm.

Yes, I found that exact solution in the meantime, too. It works indeed like a charm for the time being.
Thank you for outlining this temporary solution for others as well.

I can add a little bit of information: this solution also works well when using the latest sphinx-maven-plugin

thank you again
Jörg

@macagua
Copy link
Contributor Author

macagua commented May 22, 2019

THE STATE OF THIS PULL REQUEST

Hi guys, this improvement a time ago, it worked very well and it happened with the review of many guys here, in it moment, just a new Sphinx package release was needed to make merge into sphinx_rtd_theme theme, but the time pass and these changes dont work again because it is out of date with the base branch, then it will need to give it time and write the code for it can be accepted and merged, I hope in the next weeks dedicate time to this update this improvement.

P.D .: Really I hope in this time, the imporvements can be pass and merge more faster that in the past.

@agjohnson
Copy link
Collaborator

For me, the main problem holding this PR back is the documentation. Frankly, it's too dense of a guide for our package. I don't think we need this amount of depth in our own documentation.

Here's Sphinx's contributor guide for comparison:
http://www.sphinx-doc.org/en/master/devguide.html#locale-updates

I'd prefer if ours was similarly brief and easy to digest, otherwise folks won't know what to do with translations (myself included). I understand that maybe a guide that is sufficiently thorough is missing from the Babel community, but I don't think this package is the place to explain these concepts.

Ultimately, Read the Docs uses Transifex for all of our translations, and we'll move this package into our Transifex pattern as soon as it's ready. This masks the need to know how gettext tooling works under the hood -- including mo, po, and pot files -- and hopefully this is a lot easier for translators.

We'll have more time scheduled towards this and Sphinx 2 compatibility next month, but I can say that I'm 👍 on babel usage and most of the changes here. For me, it's simply the documentation where I get lost at and a bit overwhelmed. If we can dial this PR back on docs, this PR will be about ready to merge.

And thanks @macagua for sticking with this PR! I'm sorry this has been mostly off our roadmap for a long while, but we'll get back to guiding this a bit more as we focus on Sphinx 2 compatibility.

@agjohnson agjohnson changed the title Added localization language support #403 Added localization language support Jul 16, 2019
agjohnson added a commit that referenced this pull request Jul 17, 2019
This builds on top of #405, addressing the outstanding review feedback. It:

* Moves workflow to our standard Transifex workflow, drops recommendation for
  running babel commands by hand
* Configures Transifex
* Moves all of the commands needed to maintain translations into Grunt
* Sets up docs for translation testing
* Covers installation in docs better
* Drops recommendation for installation through submodules
* Drops superfluous translation documentation
* Cleans up some of the code
* Updates a lot of related documentation
* Updates files at Transifex and brings in full translations back to the
  translation files in the repository
agjohnson added a commit that referenced this pull request Jul 17, 2019
This builds on top of #405, addressing the outstanding review feedback. It:

* Moves workflow to our standard Transifex workflow, drops recommendation for
  running babel commands by hand
* Configures Transifex
* Moves all of the commands needed to maintain translations into Grunt
* Sets up docs for translation testing
* Covers installation in docs better
* Drops recommendation for installation through submodules
* Drops superfluous translation documentation
* Cleans up some of the code
* Updates a lot of related documentation
* Updates files at Transifex and brings in full translations back to the
  translation files in the repository
@agjohnson
Copy link
Collaborator

I've moved to addressing the outstanding PR feedback in #778. I automated more of the operations and removed the documentation on manually running babel commands. I'll aim to merge #778 into a common branch, and then deal with rebasing on master after.

@agjohnson agjohnson merged commit f9a2fe4 into readthedocs:master Jul 25, 2019
@agjohnson
Copy link
Collaborator

This work was merged in #793 🎉

Thanks again to @macagua for the effort and guidance on this, I'm quite happy with the pattern and I think Babel finally makes sense to me. This is now also a reproducible pattern for some of our other packages.

If you are interested in translating, be sure to visit https://www.transifex.com/readthedocs/sphinx-rtd-theme

We're happy to have translators and reviewers added to teams there and the number of strings to translate is very manageable.

@NicolasCaous
Copy link

How do I configure my project to use the translation feature mentioned in this issue?

I have my conf.py setting the language variable like so: language = 'pt_BR', but the theme isn't translating.

It still shows up like this:

Built with Sphinx using a theme provided by Read the Docs.

But I was expecting something like this:

Contruído com Sphinx usando um theme fornecido por Read the Docs.

However, some of the content is translated (I guess what is not in the theme but in Sphinx itself is being translated).

Can someone help a poor soul?

@derdennis
Copy link

Hey, Nicolas,

it seems my answer from May 2019 still holds true:

I don't know anything about the current work on this, but it is possible to use this functionality right now.

The folder locales in the source directory of your Sphinx project gets automagically searched for translations. If it is not there just create it. The needed structure looks like this:

source/locales/$LANG/LC_MESSAGES/sphinx.po

Where the $LANG part is a language identifier like it, ru or de.

If you want to use other folders than locales you can set the locale_dirs value in your conf.py as seen here.

I used this technique, copied in large parts from this comment, successfully with my German sphinx.po from here. The folder in locales is therefore named de.

Works like a charm.

I guess it did not work with the theme part for you, because there is no Portuguese translation folder, neither pt nor pt_br available in the locale-folder of the rtd_theme right now. You could try with nl or ru and check if everything turns Dutch or Russian and then add the Portuguese translation.

Hope that helps!

@Blendify Blendify modified the milestones: 1.1, 1.0 Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: work in progress Pull request is not ready for full review
Projects
None yet
Development

Successfully merging this pull request may close these issues.