Reference language selector #60
Conversation
I extracted the minimal feature set I could so that integration with newer upstream commits are easier.
That commit if v0.6.6.
That's (I think), the last commit for v0.6.7 (versionning in this project is hazy).
There was a conflict with rosetta/views.py, but it was easily resolved.
The url name was unquoted.
Also, made the default ref lang 'msgid' instead of 'fr'.
This way, it's possible to disable the feature. Also, added a description of the feature in the README.
|
Thanks Virgil, this looks awesome. Unfortunately I've been getting nasty IOErrors (thrown by polib.py:1313) whenever I try to select a ref language, probably because my test PO files are rubbish. I think ideally we should check whether the reference file is valid (i.e. if it passes loading into polib) before the option is displayed in the select box. What do you think? |
|
Yeah, sure. I'd have to try to reproduce the error first. Do you know specifically what kind of rubbish is causing this error? EDIT: ...or I guess whatever make a PO file invalid. yeah, I can figure that out myself. |
|
Well, for starters I had this with a completely missing PO file: in my testproject (in the repo) I have Japanese defined in my settings's LANGUAGES, but there is no PO file for Japanese (to test some edge cases) and selecting it from the list triggered the problem. |
|
Ah, yeah, languages that don't exist. That makes more sense. I was testing garbage in PO files, but that triggers many errors in django-rosetta that are out of the scope of this pull request. In fact, the ref selector should use the list_languages() function instead of the LANGUAGES settings. This way, buggy languages won't show up. |
|
(oh, I meant extract the language filtering logic in list_languages) |
|
Filtering the languages turned out to be a bit more complicated than I expected. I just fixed the crash. Do you think it's acceptable to display them, but just silently revert to MSGID when the PO can't be loaded? Wrong/missing POs is a rather rare occurrence in production system anyway... |
Since render_to_response() in the home() view no longer passes the local scope as context, we have to explicitly set ENABLE_REFLANG in the template context.
|
I've updated my pull request to the latest developments from the develop branch. Just to be sure: Is there anything I haven't done that holds this pull request from being merged? |
|
Thanks, I'll review your updated PR, write some unit tests and eventually merge it. |
Conflicts: README.rst rosetta/views.py
This crash was introduced by the merge with mbi/develop.
|
I've merged again with the develop branch and this time I even threw in a testcase :) |
The local scope is no longer used to build the template context. It is now explicitely built.
Conflicts: rosetta/conf/settings.py rosetta/tests/__init__.py rosetta/urls.py rosetta/views.py
Also, fixed the reflang feature, because the tests didn't pass anymore. references #60
|
I've merge the PR with the latest develop branch and made sure that all test pass with the Is there anything else I can do to make the PR move forward? |
|
By the way, what about introducing a mocking library into the test suite? The current way of reverting old settings in the tests is a bit flaky/inelegant. If I were to submit a PR to that effect would that be welcomed? |
|
@mbi, sorry for the repeated nudge, but is there anything I can do to help this PR go forward (other than repeatedly update from |
|
Apologies if this has taken so long, thanks for the awesome feature 👍 |
A couple of months ago, we added a reference language selector in django-rosetta and it's been sitting in our private fork of it since then. We thought you might like it, so we've cleaned it up and pushed it here.
The feature adds a Reference language selector to the translation interface. This way, instead of having only the option to show the msgid to translate, you can also have the translated version (if you already have other translations) of the msgid. That's useful if you have translators that don't speak your base language.