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

Don't scrape unnecessary strings for i18n #3438

Merged
merged 1 commit into from
Apr 28, 2014
Merged

Conversation

sarina
Copy link
Contributor

@sarina sarina commented Apr 23, 2014

@auraz can you take a look at this?

@auraz
Copy link
Contributor

auraz commented Apr 23, 2014

Result is:

#: lms/templates/video.html:128 lms/templates/video.html:129
msgid "display_name"
msgstr ""

What should be translated: values

    transcript_download_format = String(
        help="Transcript file format to download by user.",
        scope=Scope.preferences,
        values=[
            {"display_name": "SubRip (.srt) file", "value": "srt"},
            {"display_name": "Text (.txt) file", "value": "txt"}
        ],
        default='srt',
    )

So as they are not translated, I think internationalization may be removed from line 128 at all.
And added to {"display_name": _("SubRip (.srt) file"), "value": "srt"},
Right?
Thank you for catching this.

@sarina
Copy link
Contributor Author

sarina commented Apr 23, 2014

Hmm. Unfortunately we don't yet have a way to do i18n for XModule fields (this is defined in 'common/lib/xmodule/xmodule/video_module/video_xfields.py' yes?)

We could do something fancy for this particular file though, if the display_names in common/lib/xmodule/xmodule/video_module/video_xfields.py are shown in the LMS. Let me know and I can code something up for you to look at.

@auraz
Copy link
Contributor

auraz commented Apr 23, 2014

@sarina: "this is defined in 'common/lib/xmodule/xmodule/video_module/video_xfields.py' yes?"
yes

@auraz
Copy link
Contributor

auraz commented Apr 23, 2014

Yes, those values, from transcript_download_format are shown in LMS.
For clarification: It is not allowed for now to write :"display_name": _("SubRip (.srt) file")?

@sarina
Copy link
Contributor Author

sarina commented Apr 25, 2014

@auraz yes, that is the case, see https://edx-wiki.atlassian.net/browse/LMS-2078

However there is a workaround, see what I've done in this PR. What I did was in the video_xfields.py, define a dummy _ so that the strings can be successfully scraped. Then, in lms/templates/video.html, we call _() on a value which is a string, and because we've got the scraped strings, the localized string can be presented. Does that make sense?

@sarina
Copy link
Contributor Author

sarina commented Apr 28, 2014

@auraz this is ready for a second look, please

@auraz
Copy link
Contributor

auraz commented Apr 28, 2014

thanks for explanation. 👍

@sarina
Copy link
Contributor Author

sarina commented Apr 28, 2014

Use this pattern if you have need to i18n more things from Fields. And don't hesitate to ask me if you encounter any weird issues and need clarification.

@auraz
Copy link
Contributor

auraz commented Apr 28, 2014

OK, thanks!

sarina added a commit that referenced this pull request Apr 28, 2014
Don't scrape unnecessary strings for i18n
@sarina sarina merged commit 7a51382 into master Apr 28, 2014
@sarina sarina deleted the sarina/fix-video-i18n branch April 28, 2014 14:45
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