Code

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#10004 closed (fixed)

Enable -c switch in xgettext call to collect translator comments

Reported by: claudep Owned by:
Component: Internationalization Version: 1.0
Severity: Keywords: makemessages
Cc: clouserw@…, martinb Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

gettext infrastructure provides for translator comments in source code (just on the line before the string to translate) to be copied into po files. However, this required the -c (--add-comment) option to be passed to xgettext. Please add this in makemessages.py ("xgettext -c -d ...").

Attachments (4)

makemessages.py.diff (4.1 KB) - added by martinb 4 years ago.
Patch for makemessages.py: Call xgettext with -c/--add-comments
comment-extraction.patch (7.3 KB) - added by claudep 3 years ago.
Add comment extraction to makemessages with L10n keyword
comment-extraction-2.patch (2.9 KB) - added by claudep 3 years ago.
Comment extraction without tests refactoring
comment-extraction-3.patch (4.8 KB) - added by claudep 3 years ago.
Comment extraction with documentation

Download all attachments as: .zip

Change History (23)

comment:1 Changed 5 years ago by mtredinnick

  • Component changed from django-admin.py to Internationalization
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to mtredinnick
  • Patch needs improvement unset
  • Status changed from new to assigned

I thought there was already a ticket open for this, but I can't find it now. So maybe it's just one of those things that's been on my TODO list for ages. We should certainly do something like this. We need to specify (and document) a tag to mark the comments to include, since there are lots of places where there are comments preceding translation calls that are not at all relevant to the localisation process.

comment:2 Changed 5 years ago by mtredinnick

  • Owner mtredinnick deleted
  • Status changed from assigned to new

Whoops. Didn't mean to assign this to me. Anybody can do this with care and research.

comment:3 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 4 years ago by ramiro

[12296] is a first step in this direction.

comment:5 Changed 4 years ago by clouserw

  • Cc clouserw@… added

comment:6 Changed 4 years ago by clouserw

--add-comments accepts an optional tag which would filter the current comments and prevent needing to go through them all manually. If you set a standard prefix for developer comments it makes extraction easy. For example:

# L10n: This variable is a button

would be extracted with

xgettext --add-comments=L10n

Changed 4 years ago by martinb

Patch for makemessages.py: Call xgettext with -c/--add-comments

comment:7 Changed 4 years ago by martinb

  • Cc martinb added

The patch I just added is not tested nor do I have any idea whether this is the way to go. Could please somebody have a look at it? TIA.

comment:8 follow-up: Changed 4 years ago by clouserw

I haven't tested it either (it looks ok) but your help text is unclear to me. "place comment block with TAG (or those preceding keyword lines) in output file" - That sounds like it'll grab comments with the tag from anywhere, whereas the comment still needs to be directly before the call to gettext. Might just be the way I'm reading it, but I think this is more clear: Include comments directly before gettext calls in output file. If TAG is specified only comments matching TAG will be included.

comment:9 in reply to: ↑ 8 Changed 4 years ago by anonymous

Replying to clouserw:

I haven't tested it either (it looks ok) but your help text is unclear to me.

I stole this text directly from the xgettext manual page (or help output?) without change.

Include comments directly before gettext calls in output file. If TAG is specified only comments matching TAG will be included.

Yes, this sounds better.

comment:10 Changed 4 years ago by clouserw

  • milestone set to 1.2

comment:11 Changed 4 years ago by ubernostrum

  • milestone changed from 1.2 to 1.3

This feels too much like a feature request to be on the 1.2 milestone at this point.

Changed 3 years ago by claudep

Add comment extraction to makemessages with L10n keyword

comment:12 Changed 3 years ago by claudep

  • Has patch set
  • Needs documentation set

When reviewing martinb's patch, I wonder what value we add in making the addcomments switch optional and the extraction keyword configurable.

I have proposed a new patch, much simpler, which simply extract comments with the L10n keyword. Extracting translator comments should never be optional IMHO.

About tests, it's slightly messed up in the patch because I reorganized them to not require a new class for each type of test. The only real new test is the test_comments_extractor method.

comment:13 follow-up: Changed 3 years ago by jezdez

I certainly think this is a neat idea, but don't really like the literal that triggers the comment adoption, "L10n". Not having seen this elsewhere, is this again a standard somehow?

The reorganization of the tests isn't really necessary and actually makes it harder to backport single tests to 1.2.X. I'd appreciate it if that could be reverted.

comment:14 in reply to: ↑ 13 Changed 3 years ago by clouserw

I certainly think this is a neat idea, but don't really like the literal that triggers the comment adoption, "L10n". Not having seen this elsewhere, is this again a standard somehow?

There is no standard that I know of. The goal is something short, unique, and relevant; "L10n" fits the bill. We started using it at Mozilla years ago and continue to do so.

Changed 3 years ago by claudep

Comment extraction without tests refactoring

comment:15 Changed 3 years ago by claudep

Patch revised. I have no personal preference for the comment keyword, and I don't think there is any standard about it. I've seen "xgettext:" (don't like it at all), "Translators:" (a bit verbose?).

Changed 3 years ago by claudep

Comment extraction with documentation

comment:16 Changed 3 years ago by claudep

  • Needs documentation unset

comment:17 Changed 3 years ago by jezdez

comment:18 Changed 3 years ago by jezdez

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

(In [14595]) Fixed #10004 and #12320 -- Enabled the makemessages management command to collect comments for translators that start with the "Translators" keyword. Thanks for the report and patches, martinb and Claude Paroz.

comment:19 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.