Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#7606 closed enhancement (fixed)

[patch] Implement i18n support

Reported by: hasienda Owned by: Blackhex
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 hasienda 4 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 hasienda 4 years ago.
a proposal on more intuitive permission inheritance - not a requirement at all
chg-titles.patch (2.8 KB) - added by hasienda 4 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 hasienda 4 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 hasienda 4 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 hasienda 4 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 hasienda 4 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 hasienda 4 years ago.
inital message catalog template extraction, German translations
pre-0.12-branch.patch (26.1 KB) - added by Blackhex 4 years ago.
Pre 0.12 branch split reviewed patch.
l18n.patch (32.8 KB) - added by Blackhex 4 years ago.

Download all attachments as: .zip

Change History (31)

comment:1 Changed 4 years ago by hasienda

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 4 years ago by Blackhex

  • Status changed from new to assigned

OK, please attach them then here.

Changed 4 years ago by hasienda

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

Changed 4 years ago by hasienda

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

Changed 4 years ago by hasienda

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

Changed 4 years ago by hasienda

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

Changed 4 years ago by hasienda

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

Changed 4 years ago by hasienda

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 4 years ago by hasienda

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 4 years ago by hasienda

inital message catalog template extraction, German translations

comment:3 Changed 4 years ago by hasienda

  • Summary changed from Implement i18n support to [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 4 years ago by Blackhex

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

comment:5 Changed 4 years ago by Blackhex

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 4 years ago by Blackhex

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

Changed 4 years ago by Blackhex

Pre 0.12 branch split reviewed patch.

comment:7 Changed 4 years ago by Blackhex

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 4 years ago by Blackhex

Now you have the permission to commit the patch.

comment:9 Changed 4 years ago by hasienda

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 4 years ago by 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.

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 4 years ago by Blackhex

comment:11 Changed 4 years ago by Blackhex

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 4 years ago by hasienda

   <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 4 years ago by hasienda

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 4 years ago by 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.

comment:15 follow-up: Changed 4 years ago by hasienda

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 ; follow-up: Changed 4 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 4 years ago by hasienda

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 4 years ago by hasienda

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 4 years ago by Blackhex

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 4 years ago by Blackhex

  • Resolution set to fixed
  • Status changed from assigned to closed

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 4 years ago by hasienda

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.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.