Thread: pgindent messing up "translator: " comments
I just noticed that this comment got reindented by pgindent
(xlog.c, line 3226 in REL9_1_STABLE): /* * translator: First %s represents a recovery.conf parameter name like
* "recovery_end_command", and the 2nd is the value of that parameter. */ ereport((signaled && failOnSignal) ?
FATAL: WARNING, (errmsg("%s \"%s\": return code %d", commandName, command, rc)));
Sure enough, the resulting POT entry does not have the necessary
comment:
#: /pgsql/source/REL9_1_STABLE/src/backend/access/transam/xlog.c:3230
#, c-format
msgid "%s \"%s\": return code %d"
msgstr ""
I think the proper fix would be to use the /*---- trick, such as in
postmaster.c:
/*------ translator: %s is a noun phrase describing a child process, such as "server process" */
(errmsg("%s (PID %d) exited with exit code %d", procname, pid, WEXITSTATUS(exitstatus))));
It seems to me that we should alert if pgindent does anything to a
comment line containing "translator:".
--
Álvaro Herrera <alvherre@alvh.no-ip.org>
Alvaro Herrera wrote:
> I just noticed that this comment got reindented by pgindent
> (xlog.c, line 3226 in REL9_1_STABLE):
> /*
> * translator: First %s represents a recovery.conf parameter name like
> * "recovery_end_command", and the 2nd is the value of that parameter.
> */
> ereport((signaled && failOnSignal) ? FATAL : WARNING,
> (errmsg("%s \"%s\": return code %d", commandName,
> command, rc)));
>
> Sure enough, the resulting POT entry does not have the necessary
> comment:
>
> #: /pgsql/source/REL9_1_STABLE/src/backend/access/transam/xlog.c:3230
> #, c-format
> msgid "%s \"%s\": return code %d"
> msgstr ""
>
> I think the proper fix would be to use the /*---- trick, such as in
> postmaster.c:
>
> /*------
> translator: %s is a noun phrase describing a child process, such as
> "server process" */
> (errmsg("%s (PID %d) exited with exit code %d",
> procname, pid, WEXITSTATUS(exitstatus))));
>
> It seems to me that we should alert if pgindent does anything to a
> comment line containing "translator:".
Well, the comment adjustments happen in the C code, which is hard to
modify. We would need a wrapper that understood when it was in a C
command and add /*--- markers if the word 'translator:' appeared.
-- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB
http://enterprisedb.com
+ It's impossible for everything to be true. +
Excerpts from Alvaro Herrera's message of lun sep 05 16:21:38 -0300 2011:
> I just noticed that this comment got reindented by pgindent
> (xlog.c, line 3226 in REL9_1_STABLE):
> /*
> * translator: First %s represents a recovery.conf parameter name like
> * "recovery_end_command", and the 2nd is the value of that parameter.
> */
> ereport((signaled && failOnSignal) ? FATAL : WARNING,
> (errmsg("%s \"%s\": return code %d", commandName,
> command, rc)));
Actually, after I looked into Git history it turns out that this comment
was introduced in this way; it wasn't pgindent's fault. I checked a
couple of diffs from pgindent runs, and I found no "translator:" comment
reindented destructively. Still, it seems possible that it could happen
someday.
I will fix this one occurence.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I think the proper fix would be to use the /*---- trick, such as in
> postmaster.c:
> /*------
> translator: %s is a noun phrase describing a child process, such as
> "server process" */
> (errmsg("%s (PID %d) exited with exit code %d",
> procname, pid, WEXITSTATUS(exitstatus))));
Ugh. Are the gettext tools so broken that they force us to use that
(very ugly IMO) layout for translator: comments? Why can't we get
the tools fixed instead?
By and large, the people who put in those comments don't know about any
specialized restrictions that gettext might have on the layout of the
comment; the only documentation I've ever seen just says that the
comment has to start with "translator:":
http://developer.postgresql.org/pgdocs/postgres/nls-programmer.html
I think that if gettext can't handle the comment as it stands, that's
a gettext bug, not something that both pgindent and the human code
authors ought to be subservient to. Or at the very least, I want to see
an exact specification for what the allowed format is, and it had better
not be very magical.
regards, tom lane
Excerpts from Tom Lane's message of lun sep 05 16:43:32 -0300 2011:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > I think the proper fix would be to use the /*---- trick, such as in
> > postmaster.c:
>
> > /*------
> > translator: %s is a noun phrase describing a child process, such as
> > "server process" */
> > (errmsg("%s (PID %d) exited with exit code %d",
> > procname, pid, WEXITSTATUS(exitstatus))));
>
> Ugh. Are the gettext tools so broken that they force us to use that
> (very ugly IMO) layout for translator: comments? Why can't we get
> the tools fixed instead?
>
> By and large, the people who put in those comments don't know about any
> specialized restrictions that gettext might have on the layout of the
> comment; the only documentation I've ever seen just says that the
> comment has to start with "translator:":
> http://developer.postgresql.org/pgdocs/postgres/nls-programmer.html
Well, this is all the xgettext manpage says:
-cTAG, --add-comments=TAG place comment blocks starting with TAG and preceding keyword lines in
outputfile
I think nobody bothers to fix this because everyone else is using the
GNU indentation style, which would make the message look like this:
/* translator: %s is a noun phrase describing a child process,such as "server process" */errmsg( ... );
> I think that if gettext can't handle the comment as it stands, that's
> a gettext bug, not something that both pgindent and the human code
> authors ought to be subservient to. Or at the very least, I want to see
> an exact specification for what the allowed format is, and it had better
> not be very magical.
Hmm. I think the only other place than the above line in the manpage
where this is mentioned in the manual, is this:
http://www.gnu.org/software/gettext/manual/gettext.html#Bug-Report-Address
No mention of the format is done anywhere.
This seems related to this (unanswered) bug report:
http://savannah.gnu.org/bugs/?33451
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support