Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7606 closed enhancement (fixed)

[patch] Implement i18n support

Reported by: Steffen Hoffmann Owned by: Radek Bartoň
Priority: normal Component: ScreenshotsPlugin
Severity: normal Keywords: i18n l10n
Cc: Trac Release: 0.11

Description

We should start a 0.12 branch and work on this.

Attachments (10)

pending-bugfixes.patch (12.7 KB) - added by Steffen Hoffmann 7 years ago.
despite of name mostly creating some init() modules, just one occurance of self.config.get needed, formatting (proposal) for better readability
inheriting-perms.patch (1.2 KB) - added by Steffen Hoffmann 7 years ago.
a proposal on more intuitive permission inheritance - not a requirement at all
chg-titles.patch (2.8 KB) - added by Steffen Hoffmann 7 years ago.
make titels regarding case and wording with(out) colon more like native Trac - later i18n work relies on that exact strings
chg-sql.patch (10.6 KB) - added by Steffen Hoffmann 7 years ago.
another optional proposal: more readable SQL statements, the second hardest part in this patch stack, but not required for i18n at all
add_0.12-branch.patch (5.5 KB) - added by Steffen Hoffmann 7 years ago.
just branching of 0.12 off from 0.11 here after the previous preparation
add_i18n-basics.patch.tgz (5.5 KB) - added by Steffen Hoffmann 7 years ago.
core code and markup to bring in and setup i18n/l10n for Trac plugins, required markup for translatable strings (=msgid's), at least as far as they are not auto-detected (in Genshi HTML templates), including minor related code changes, i.e. to avoid msgid duplication - packed to circumvent SPAM t-h.o's filter
btt-label-translation.patch (5.6 KB) - added by Steffen Hoffmann 7 years ago.
workaround needed for still unknown reasons, but wouldn't get some button label text's translated otherwise - might lead to a Genshi ticket later on
extract_msg.patch (16.7 KB) - added by Steffen Hoffmann 7 years ago.
inital message catalog template extraction, German translations
pre-0.12-branch.patch (26.1 KB) - added by Radek Bartoň 7 years ago.
Pre 0.12 branch split reviewed patch.
l18n.patch (32.8 KB) - added by Radek Bartoň 7 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 7 years ago by Steffen Hoffmann

I'll provide patches based on current 0.11 branch as promised per email.

I work with Mercurial here cloning full t-h.o SVN repo, so these patches are meant to become changesets and should apply directly to current screenshotsplugin branch.

comment:2 Changed 7 years ago by Radek Bartoň

Status: newassigned

OK, please attach them then here.

Changed 7 years ago by Steffen Hoffmann

Attachment: pending-bugfixes.patch added

despite of name mostly creating some init() modules, just one occurance of self.config.get needed, formatting (proposal) for better readability

Changed 7 years ago by Steffen Hoffmann

Attachment: inheriting-perms.patch added

a proposal on more intuitive permission inheritance - not a requirement at all

Changed 7 years ago by Steffen Hoffmann

Attachment: chg-titles.patch added

make titels regarding case and wording with(out) colon more like native Trac - later i18n work relies on that exact strings

Changed 7 years ago by Steffen Hoffmann

Attachment: chg-sql.patch added

another optional proposal: more readable SQL statements, the second hardest part in this patch stack, but not required for i18n at all

Changed 7 years ago by Steffen Hoffmann

Attachment: add_0.12-branch.patch added

just branching of 0.12 off from 0.11 here after the previous preparation

Changed 7 years ago by Steffen Hoffmann

Attachment: add_i18n-basics.patch.tgz added

core code and markup to bring in and setup i18n/l10n for Trac plugins, required markup for translatable strings (=msgid's), at least as far as they are not auto-detected (in Genshi HTML templates), including minor related code changes, i.e. to avoid msgid duplication - packed to circumvent SPAM t-h.o's filter

Changed 7 years ago by Steffen Hoffmann

Attachment: btt-label-translation.patch added

workaround needed for still unknown reasons, but wouldn't get some button label text's translated otherwise - might lead to a Genshi ticket later on

Changed 7 years ago by Steffen Hoffmann

Attachment: extract_msg.patch added

inital message catalog template extraction, German translations

comment:3 Changed 7 years ago by Steffen Hoffmann

Summary: Implement i18n support[patch] Implement i18n support

Ok, just finished my work for working i18n/l10n now.

I've attached in patch order and already commented the patches above. All are automatically created by Mercurial Queues extension, but Trac has problems to show some of them, nevertheless they are there (see original format then).

Where I brought in disputable stuff, I've tried to flag and comment accordingly. No problem to rework for most of it, if you insist. However there are some restrictions that are imposed by the way Trac i18n support for plugins is meant to word. You may want to take a look at the reference documentation for adding i18n/l10n to Trac plugins.

Beware, the Genshi/Babel part is still not totally settled - we have a critical case here as well, that I needed some extra code to circumvent limitations most probably related to automatic Genshi template translations by genshi.filters.i18n.Translator for now. I've just shifted string definition and translation from the Genshi template into python code space know to provide more reliable i18n support and only import back into template afterwards.

Let me know, how you'd like to proceed. I.e., if you see need for extensive discussion on particular patches, please tell me, so I could open separate tickets for them as needed. For my own part I already happily use the patched version in production and will fix/improve, as needed for my application. At the very minimum it's already known to be safe for non-ascii strings in screenshot names, description, etc.

I'd still like to add message catalogs to our Trac plugin l10n project via Transifex. We'll certainly see contributions for other languages soon without much additional effort on plugin maintainers side, as the Trac plugin l10n project is managed by myself.

comment:4 Changed 7 years ago by Radek Bartoň

OK, thank you. I'll review the patches tomorrow.

comment:5 Changed 7 years ago by Radek Bartoň

I'm sequentially applying and reviewing the patches now.

Regarding the inheriting-perms.patch: I don't see the reason of this patch. An explanation for the former version is that when you wan't to edit or delete a screenshot you have to see it but you don't have to have rights for adding it (in case of edit) or editing it (in case of delete). SCREENSHOTS_ADMIN is just shortcut for permission to do anything.

comment:6 Changed 7 years ago by Radek Bartoň

Ehh, wan't is really nice typo :-).

Changed 7 years ago by Radek Bartoň

Attachment: pre-0.12-branch.patch added

Pre 0.12 branch split reviewed patch.

comment:7 Changed 7 years ago by Radek Bartoň

OK, I've attached the pre-0.12-branch.patch. You can commit it as soon as you'll have permission to do it. Then I'll create a 0.12 branch and review the i18n patches.

comment:8 Changed 7 years ago by Radek Bartoň

Now you have the permission to commit the patch.

comment:9 Changed 7 years ago by Steffen Hoffmann

Thanks for the pre-branch review as well as for SVN commit. Nice take on the SQL statements making them even more uniform and readable. There is definitely good stuff in this one. Nice cooperation so far. Patch committed in changeset [8663]. Will comment regarding permissions concept later.

comment:10 Changed 7 years ago by Radek Bartoň

l18n.patch is file with reviewed patches for l18n. I didn't apply the [btt-label-translation.patch] because we should find proper way how to translate such literals. I'm asking now at #trac IRC channel about it. You can commit it now, please.

I also started translation of ScreenshotPlugin to Czech and registered at Transifex (user Blackhex). Could you please add ScreenshotsPlugin (and maybe DiscussionPlugin) components to Trac Plugin L10N project and add me as member of Czech team?

Changed 7 years ago by Radek Bartoň

Attachment: l18n.patch added

comment:11 Changed 7 years ago by Radek Bartoň

I replaced the l18n.patch with a new version that fixes the problem. It was caused by the fact that implementation of _() function in template was default one. You have to import customized implementation from the plugin. Now all translations work.

comment:12 Changed 7 years ago by Steffen Hoffmann

   <xi:include href="layout.html"/>
   <xi:include href="macros.html"/>
+  <?python
+    from tracscreenshots.core import _
+  ?>
   <head>
     <title>Screenshots</title>

I love this one. Simple, straightforward - beautiful. I was really at a loss, had asked at #trac myself before, but I couldn't get a grip on this laying that near as it is obvious now. Thanks so much for lightning a bunch of torches for me. So we can happily forget about the additional fix-up patch, as I hoped for some day.

comment:13 Changed 7 years ago by Steffen Hoffmann

I'm missing the new setup.cfg file. You'll certainly fail to test and get the i18n stuff working without that configuration (extractor/translator calls, i18n catalog domain, paths). As I see nothing seriously disputable in there, I'll simply re-add the file before commit, right?

comment:14 Changed 7 years ago by Radek Bartoň

Ah, sorry, I forgot to add it to version control before doing svn diff. Please do so but change Babel version requirement to 0.9.4 because it is the current version for Ubuntu stable.

comment:15 Changed 7 years ago by Steffen Hoffmann

Replying to Blackhex:

l18n.patch is file with reviewed patches for l18n. I didn't apply the [btt-label-translation.patch] because we should find proper way how to translate such literals. I'm asking now at #trac IRC channel about it. You can commit it now, please.

Just doing a second check and local test as you see from my comment before.

I also started translation of ScreenshotPlugin to Czech and registered at Transifex (user Blackhex). Could you please add ScreenshotsPlugin (and maybe DiscussionPlugin) components to Trac Plugin L10N project and add me as member of Czech team?

Sure, it'll be a pleasure for me to add plugins. But sorry, you can't be simply a member of the Czech translator team, I needed a coordinator to bring it into life - done right now. :-)

Only I need to have a look at DiscussionPlugin, was not aware, that it already has i18n in. But OTOH it was months ago and well before Trac 0.12 release, when I last scanned all plugin code from t-h.o for traces of i18n support. I'm really glad, that there is now some progress regarding i18n.

comment:16 in reply to:  15 ; Changed 7 years ago by anonymous

Only I need to have a look at DiscussionPlugin, was not aware, that it already has i18n in. But OTOH it was months ago and well before Trac 0.12 release, when I last scanned all plugin code from t-h.o for traces of i18n support. I'm really glad, that there is now some progress regarding i18n.

Currently it doesn't have l18n support but I can write it.

comment:17 Changed 7 years ago by Steffen Hoffmann

Replying to Blackhex:

Ah, sorry, I forgot to add it to version control before doing svn diff. Please do so but change Babel version requirement to 0.9.4 because it is the current version for Ubuntu stable.

Ok, thanks for the quick response. Requirement is set in setup.py; will check that one, but I see no serious problem for that move. I've just copied that requirement form Trac 0.12, since I had no clue on my own, working with Babel > 1.0 all the time.

comment:18 in reply to:  16 Changed 7 years ago by Steffen Hoffmann

Replying to anonymous:

Only I need to have a look at DiscussionPlugin, was not aware, that it already has i18n in. But OTOH it was months ago and well before Trac 0.12 release, when I last scanned all plugin code from t-h.o for traces of i18n support. I'm really glad, that there is now some progress regarding i18n.

Currently it doesn't have l18n support but I can write it.

Patch welcome. ;-) Not kidding, you seem quite up to the task considering the way you helped me with this one. And I'll be happy to assist, if you just need a second eye.

comment:19 Changed 7 years ago by Radek Bartoň

Hmm, I quite don't understand how Transifex works. When I'm coordinator I can't submit translations while I can't be both coordinator and member :-).

Than you for letting me in anyway.

comment:20 Changed 7 years ago by Radek Bartoň

Resolution: fixed
Status: assignedclosed

Ok, I've commited (r8717) Czech translation. We are done with this ticket so I'm closing it. Thank you for your cooperation.

comment:21 Changed 7 years ago by Steffen Hoffmann

Well thank you too. I'll setup ScreenshotsPlugin as another component in our Transifex project for Trac plugin translation and add links to ScreenshotsPlugin wiki page.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Radek Bartoň.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.