Re: [HACKERS] pgindent messing up "translator: " comments

2011-09-05 Thread Alvaro Herrera
Excerpts from Tom Lane's message of lun sep 05 16:43:32 -0300 2011:
> Alvaro Herrera  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 output file

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 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgindent messing up "translator: " comments

2011-09-05 Thread Tom Lane
Alvaro Herrera  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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgindent messing up "translator: " comments

2011-09-05 Thread Alvaro Herrera
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 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgindent messing up "translator: " comments

2011-09-05 Thread Bruce Momjian
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  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] pgindent messing up "translator: " comments

2011-09-05 Thread Alvaro Herrera
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 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers