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

Many i18n improvements #2944

Merged
merged 10 commits into from
Jul 17, 2017
Merged

Conversation

ewjoachim
Copy link
Contributor

@ewjoachim ewjoachim commented Jun 10, 2017

I guess all of this goes towards #1455 but there's probably still a lot to be done.

Actually found this one while completing the French translation in Transifex :D

EDIT : added another commit for a translation specific problem. I'll probably fix the other things I see here (until someone does a review, at which point the PR will be frozen)

EDIT again : just so you know, I've finished translating all the French keys, so this PR should be final

EDIT again again : I removed trans and blocktrans tags around the project/brand name "Read the Docs", because my intimate guess is that there's no reason for this to have been translated in the first place. (Imagine people in an international conference trying to understand they're talking about the same service with different names). I couldn't find a prior discussion for the matter. Feel free to say so if it wasn't a good idea.

@ewjoachim ewjoachim force-pushed the count-translation branch 4 times, most recently from 21d9e66 to 502a90d Compare June 11, 2017 09:49
@ewjoachim
Copy link
Contributor Author

Just for the lol of it, this is what we're currently asking our volunteers to translate :D
capture d ecran 2017-06-11 a 11 50 34
for something that should be 2 sentences with no tags 🤣

@ewjoachim ewjoachim force-pushed the count-translation branch from a8cdb08 to bc09c9f Compare June 11, 2017 11:03
@ewjoachim ewjoachim changed the title Use blocktrans with count instead of manually counting Many i18n improvements Jun 11, 2017
@@ -41,7 +43,7 @@
{% if project.skip %}
<p class="build-failure">
{% blocktrans %}
Your project is currently disabled for abuse of the system.
Your project is currently disabled for abuse of the system.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My text editor did this automatically. Do you want me to undo it or do it as a separate commit ? From my point of view, it's better this way.

@ewjoachim ewjoachim force-pushed the count-translation branch from 4e559fb to 0c2e06f Compare June 13, 2017 20:19
@ericholscher
Copy link
Member

Ouch! Thanks for this work. I'll take a look at it in the next couple days, but it looks pretty straight forward.

Is there more that we could be doing to make translations better? This is something that I don't have a lot of experience with, and would love to have help with.

@ewjoachim
Copy link
Contributor Author

ewjoachim commented Jun 14, 2017

It's well documented that one who wishes to help with translations should go to transifex so that's a good point. I've seen projects where translation is really reserved to developers, and this project is much better so that's clearly good.

That being said, there are a few things I can think of :) Note : most of these are from general experience of translator in code projects, which I've done a a good bit but is neither my trade nor something I've extensively done.

Non technical

  • Setting up a glossary, defining terms used throughout the app in a consistent way (ex. "promo" vs "ad", both are used)
  • Writing clear translation rules (like "the name Read the Docs is not to be translated"). Also, a (possibily per-language) stance on how gender is to be treated would be great. In French, we can use "usual" language that's male-oriented, turn sentences around to avoid any assumptions on the reader's gender but it kinda breaks the reading flow, or use inclusive language ("s⋅he") which we see more and more in french tech communities but for which there is no official grammatical rules, and adds more questions (should we use ., , -, () etc)
  • For every translatable string, if there can be a uncertainty as of how to read it ("is it a noun or a verb ?", add context. Also, for strings that carry a meaning / an emotion above the words, add context (ex. "Keep documenting, Read the Docs" in the footer, we might want something more clever than a word-to-word translation there)
  • When generating the PO file, one should always read it and try to imagine if the words alone are enough for a clear understanding of what this means. Technical people may be able to look at the code for clues (as I did), but if we support non-technical people contributing to translations, it should not be a requirement.

(this being said, in 95% of the cases, common sense and the fact sentences in a same file are usually close from one another in the translation are enough to derive context)

Note that all those things are not necessary, but if absent, they force the translator to make their choice, which can lead to a bit of inconsistency.

Technical

  • All cases where pluralization is made should always use the gettext pluralization. More generally, as few attempts as possible should be made to dynamically construct a sentence from word parts in the code. Whole sentence, even with multiple placeholders, should be translated as a block.
  • And on the other way, translated strings should never contain unnecessary html. I think a good rule of thumb whould be "inline html is ok, blocks are not". Ideally, a translation string is for a single full sentence or a paragraph. If there's some kind of (not inline) layout, it's better to split it.
  • if all the blocktrans used trimmed (which should definitely be the default), there would be a lot less \n everywhere around, which Transifex is really pesky about. (if we do this, we can remove those \n from the .po file so that we don't have to retranslate everything)

Now all that being said, the community has done an excellent job so far on setting up a welcoming i18n effort on this project, so even if there is still things to do (and there will always be), congratulations are due !

@ewjoachim
Copy link
Contributor Author

ewjoachim commented Jun 19, 2017

(should I continuously rebase and remove conflicts or wait for the review to do that ?)

@agjohnson
Copy link
Contributor

@ewjoachim The conflicts look to be minor, I can probably handle the conflicts during review. I'm going to try to review these changes here today.

I'm really happy to have some direction here, this is all great feedback! I think a lot of this information can make it into a translation styleguide of sorts -- though we don't have any styleguides for our code/etc yet. Ideally, this is all organization-wide, but I think adding to the documentation here is a great start.

@ewjoachim
Copy link
Contributor Author

If there's anything I can do to help, let me know :). If I just need to be more patient, I'm ok, with that too :)

@ericholscher
Copy link
Member

If you can rebase this to fix the conflicts, I'll be happy to merge it! Sorry it's been sitting for so long.

@ericholscher ericholscher merged commit 0c2e06f into readthedocs:master Jul 17, 2017
@ericholscher
Copy link
Member

Went ahead and fixed the conflicts 👍

@ewjoachim
Copy link
Contributor Author

Ah thank you ! Sorry, didn't have the time to do it then, and then it fell at the bottom of a todo list. Thank you a lot for doing it.

I can't find in the docs info on the process for uploading to transifex (we should add that) so I'll ask here: when do you think this will be reflected on the po files ?

@ewjoachim ewjoachim deleted the count-translation branch July 17, 2017 21:30
@ericholscher
Copy link
Member

https://github.com/rtfd/readthedocs.org/blob/master/fabfile.py#L9 is how I upload them. I am doing the process now. It hasn't been done in a long time, and I'm guessing we still have a lot of bits that aren't being marked for translation properly. It would be great to have folks on the core team who are passionate about this.

We have a bit of docs on this here: http://docs.readthedocs.io/en/latest/i18n.html#administrative-tasks -- but could likely be updated.

@ewjoachim ewjoachim mentioned this pull request Aug 5, 2018
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.

3 participants