Opened 15 years ago

Closed 13 years ago

Last modified 12 years ago

#10004 closed (fixed)

Enable -c switch in xgettext call to collect translator comments

Reported by: Claude Paroz 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: no UI/UX: no

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 14 years ago.
Patch for makemessages.py: Call xgettext with -c/--add-comments
comment-extraction.patch (7.3 KB ) - added by Claude Paroz 13 years ago.
Add comment extraction to makemessages with L10n keyword
comment-extraction-2.patch (2.9 KB ) - added by Claude Paroz 13 years ago.
Comment extraction without tests refactoring
comment-extraction-3.patch (4.8 KB ) - added by Claude Paroz 13 years ago.
Comment extraction with documentation

Download all attachments as: .zip

Change History (23)

comment:1 by Malcolm Tredinnick, 15 years ago

Component: django-admin.pyInternationalization
Owner: changed from nobody to Malcolm Tredinnick
Status: newassigned

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 by Malcolm Tredinnick, 15 years ago

Owner: Malcolm Tredinnick removed
Status: assignednew

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

comment:3 by Alex Gaynor, 15 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Ramiro Morales, 14 years ago

[12296] is a first step in this direction.

comment:5 by Wil Clouser, 14 years ago

Cc: clouserw@… added

comment:6 by Wil Clouser, 14 years ago

--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

by martinb, 14 years ago

Attachment: makemessages.py.diff added

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

comment:7 by martinb, 14 years ago

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 by Wil Clouser, 14 years ago

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.

in reply to:  8 comment:9 by anonymous, 14 years ago

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 by Wil Clouser, 14 years ago

milestone: 1.2

comment:11 by James Bennett, 14 years ago

milestone: 1.21.3

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

by Claude Paroz, 13 years ago

Attachment: comment-extraction.patch added

Add comment extraction to makemessages with L10n keyword

comment:12 by Claude Paroz, 13 years ago

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 by Jannis Leidel, 13 years ago

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.

in reply to:  13 comment:14 by Wil Clouser, 13 years ago

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.

by Claude Paroz, 13 years ago

Attachment: comment-extraction-2.patch added

Comment extraction without tests refactoring

comment:15 by Claude Paroz, 13 years ago

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?).

by Claude Paroz, 13 years ago

Attachment: comment-extraction-3.patch added

Comment extraction with documentation

comment:16 by Claude Paroz, 13 years ago

Needs documentation: unset

comment:17 by Jannis Leidel, 13 years ago

comment:18 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: newclosed

(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 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top