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

Fix translation of WTForms strings (simplified again) #13

Merged
merged 3 commits into from
Dec 10, 2013
Merged

Fix translation of WTForms strings (simplified again) #13

merged 3 commits into from
Dec 10, 2013

Conversation

mdxs
Copy link
Member

@mdxs mdxs commented Dec 10, 2013

This resolves the #6 "Strings that are in WTForms are not translated" issue.

Note: this supersedes the previous #12 (including feedback provided there) and #10 pull requests.

@mdxs
Copy link
Member Author

mdxs commented Dec 10, 2013

Steps for testing implementation:

  • Extract strings and initialize translations:
./run.py -e
./run.py -i
  • Edit main/translations/ru/LC_MESSAGES/messages.po
  • Update the msgid "This field is required." and msgstr "" directly below it to read:
msgid "This field is required."
msgstr "Thisa fielda isa requireda. (ru)"
  • With my apologies to Russian speakers, this is just for testing...
  • Compile the translations and start the local development server:
./run.py -b
./run.py -s
  • Open your browser and go to http://localhost:8080
  • Select "Sign in" and use the "Sign in with Google" (mock-up)
    • Follow the defaults to get a simple test account and click "Login"
  • Select "Profile Settings" under your test account
  • Empty the "Name" field and click "Update Profile"
    • Assuming you're using the default en locale (the British flag) or any other non-Russian locale, you will see "This field is required."
  • Change to the ru locale by selecting the Russian flag (probably the last one in the list)
  • Again empty the "Name" field and click "Update Profile"
    • Now you should see "Thisa fielda isa requireda. (ru)" below the Name field

@lipis
Copy link
Member

lipis commented Dec 10, 2013

I'm afraid you didn't update two more forms that are in the project.. profile and feedback

@mdxs
Copy link
Member Author

mdxs commented Dec 10, 2013

Thanks for spotting that, I've too many branches... waiting for opportunity to prune :-)

@mdxs mdxs mentioned this pull request Dec 10, 2013
@lipis
Copy link
Member

lipis commented Dec 10, 2013

I'm about to merge that... tested it and it worked nicely..! Do you remember the reference on where you find on how to resolve that?

lipis added a commit that referenced this pull request Dec 10, 2013
Fix translation of WTForms strings
@lipis lipis merged commit 7551db3 into gae-init:master Dec 10, 2013
@lipis
Copy link
Member

lipis commented Dec 10, 2013

I simplified it a bit: 2e870bd :) check you'll spot something strange...

@lipis
Copy link
Member

lipis commented Dec 10, 2013

Did you try deploying that? Does that still works?! :|

@mdxs
Copy link
Member Author

mdxs commented Dec 10, 2013

It is based on http://wtforms.readthedocs.org/en/1.0.5/i18n.html#writing-your-own-translations-provider and on an approach I noted in wtforms.ext.i18n. The latter aimed to deploy/support pre-translated messages for WTForms, which in trying to use didn't work: most likely due to the main/lib.zip approach, where some Python builtin gettext.find and/or other translations stuff couldn't find/load the proper WTForms messages in the zip-file.

Did you try deploying that? Does that still works?! :|

I've just deployed my proposed PR version on appspot.com and that worked (with the "Steps for testing implementation:" described above); I've not (yet) checked your further modification.

@lipis
Copy link
Member

lipis commented Dec 10, 2013

Thanks.. It works.. you can simply press the blue button here: http://babel.gae-init.appspot.com/l/el/?next=/feedback/ ;)

@mdxs
Copy link
Member Author

mdxs commented Dec 11, 2013

I just confirmed that your further modification works on appspot.com as well.

I'm not 100% sure why though? I thought the try-except and return None was a needed safeguard in case any of the removed (previous) if-then-returns; with the try-except being easier to understand (with my understanding that try-except is a good approach in Python; as confirmed in http://stackoverflow.com/a/7604717/2315612).

Anyway, your link does simplify my test procedure a lot... too bad my localhost gives me a 418 on http://localhost:8080/l/ru/?next=/feedback/ (too late to figure it out now... need sleep)

@mdxs
Copy link
Member Author

mdxs commented Dec 11, 2013

Your test site also shows that it handles unknown locales, like http://babel.gae-init.appspot.com/l/nl/?next=/feedback/ defaults to the en (lacking my perfectly fine nl_NL translations...)

@mdxs
Copy link
Member Author

mdxs commented Dec 11, 2013

Ah, now I know why "I'm a teapot" / 418... need to set the "Feedback Email" on the "Admin Config" first.

@mdxs mdxs deleted the fix_wtforms_translations branch December 11, 2013 00:11
@lipis
Copy link
Member

lipis commented Dec 11, 2013

Yes the ?next= is nice and of course it works on the sign-in page as well..

as for the simplified version.. I just skipped checking the wtforms_translations which I know by fact that is not there.. of course if the translation directory would be missing it might throw an exception.. but it should be missing in a first place, cause the whole thing is not going to make sense.. so it's fine.. who ever will get an error.. they should be able to figure it out.

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