pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Mark the second argument of pg_log as the translatable string in
11+ messages / 6 participants
[nested] [flat]

* pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-08 02:06  Fujii Masao <fujii@postgresql.org>
  0 siblings, 2 replies; 11+ messages in thread

From: Fujii Masao @ 2015-04-08 02:06 UTC (permalink / raw)
  To: pgsql-committers

Mark the second argument of pg_log as the translatable string in nls.mk.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/b216ad7bf1a9308c97d2032d4793010e8c8aa7ec

Modified Files
--------------
src/bin/pg_rewind/nls.mk |    2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-08 04:35  Michael Paquier <michael.paquier@gmail.com>
  parent: Fujii Masao <fujii@postgresql.org>
  1 sibling, 1 reply; 11+ messages in thread

From: Michael Paquier @ 2015-04-08 04:35 UTC (permalink / raw)
  To: Fujii Masao <fujii@postgresql.org>; +Cc: pgsql-committers

On Wed, Apr 8, 2015 at 11:06 AM, Fujii Masao <fujii@postgresql.org> wrote:
> Mark the second argument of pg_log as the translatable string in nls.mk.

nls.mk is still missing file_ops.c in GETTEXT_FILES as it contains
some calls to pg_fatal.
-- 
Michael


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-08 04:52  Fujii Masao <masao.fujii@gmail.com>
  parent: Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 0 replies; 11+ messages in thread

From: Fujii Masao @ 2015-04-08 04:52 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Fujii Masao <fujii@postgresql.org>; pgsql-committers

On Wed, Apr 8, 2015 at 1:35 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Apr 8, 2015 at 11:06 AM, Fujii Masao <fujii@postgresql.org> wrote:
>> Mark the second argument of pg_log as the translatable string in nls.mk.
>
> nls.mk is still missing file_ops.c in GETTEXT_FILES as it contains
> some calls to pg_fatal.

Oh, sorry. I was wrongly thinking that the Heikki's recently changes
to nls.mk fixed that....

Fixed. Thanks!

Regards,

-- 
Fujii Masao


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-12 01:08  Peter Eisentraut <peter_e@gmx.net>
  parent: Fujii Masao <fujii@postgresql.org>
  1 sibling, 1 reply; 11+ messages in thread

From: Peter Eisentraut @ 2015-04-12 01:08 UTC (permalink / raw)
  To: Fujii Masao <fujii@postgresql.org>; pgsql-committers

On 4/7/15 10:06 PM, Fujii Masao wrote:
> Mark the second argument of pg_log as the translatable string in nls.mk.

gettext (msgmerge) is unhappy about this because

po/pg_rewind.pot:501: warning: internationalized messages should not
contain the '\r' escape sequence



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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-12 01:17  Alvaro Herrera <alvherre@2ndquadrant.com>
  parent: Peter Eisentraut <peter_e@gmx.net>
  0 siblings, 1 reply; 11+ messages in thread

From: Alvaro Herrera @ 2015-04-12 01:17 UTC (permalink / raw)
  To: Peter Eisentraut <peter_e@gmx.net>; +Cc: Fujii Masao <fujii@postgresql.org>; pgsql-committers

Peter Eisentraut wrote:
> On 4/7/15 10:06 PM, Fujii Masao wrote:
> > Mark the second argument of pg_log as the translatable string in nls.mk.
> 
> gettext (msgmerge) is unhappy about this because
> 
> po/pg_rewind.pot:501: warning: internationalized messages should not
> contain the '\r' escape sequence

What pg_basebackup's progress_report() does is have the message in the
translatable part not include the \r; the \r is in a separate fprintf()
call.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-12 06:03  Michael Paquier <michael.paquier@gmail.com>
  parent: Alvaro Herrera <alvherre@2ndquadrant.com>
  0 siblings, 2 replies; 11+ messages in thread

From: Michael Paquier @ 2015-04-12 06:03 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Peter Eisentraut <peter_e@gmx.net>; Fujii Masao <fujii@postgresql.org>; pgsql-committers

On Sun, Apr 12, 2015 at 10:17 AM, Alvaro Herrera wrote:
> What pg_basebackup's progress_report() does is have the message in the
> translatable part not include the \r; the \r is in a separate fprintf()
> call.

Like the attached then.
-- 
Michael

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

Attachments:

  [text/x-patch] 20150412_pgrewind_fix.patch (512B, ../../CAB7nPqQP8NU3WztTG8Snd4k2S5dLx1CN0dRCzcuFF1GuyqJATQ@mail.gmail.com/2-20150412_pgrewind_fix.patch)
  download | inline diff:
diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
index aba12d8..3e2dc76 100644
--- a/src/bin/pg_rewind/logging.c
+++ b/src/bin/pg_rewind/logging.c
@@ -134,7 +134,8 @@ progress_report(bool force)
 	snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
 			 fetch_size / 1024);
 
-	pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied\r",
+	pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied",
 		   (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
 		   percent);
+	printf("\r");
 }


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-13 04:43  Fujii Masao <masao.fujii@gmail.com>
  parent: Michael Paquier <michael.paquier@gmail.com>
  1 sibling, 0 replies; 11+ messages in thread

From: Fujii Masao @ 2015-04-13 04:43 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Peter Eisentraut <peter_e@gmx.net>; Fujii Masao <fujii@postgresql.org>; pgsql-committers

On Sun, Apr 12, 2015 at 3:03 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sun, Apr 12, 2015 at 10:17 AM, Alvaro Herrera wrote:
>> What pg_basebackup's progress_report() does is have the message in the
>> translatable part not include the \r; the \r is in a separate fprintf()
>> call.
>
> Like the attached then.

Pushed. Thanks a lot!

Regards,

-- 
Fujii Masao


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-13 17:17  Alvaro Herrera <alvherre@2ndquadrant.com>
  parent: Michael Paquier <michael.paquier@gmail.com>
  1 sibling, 1 reply; 11+ messages in thread

From: Alvaro Herrera @ 2015-04-13 17:17 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: Peter Eisentraut <peter_e@gmx.net>; Fujii Masao <fujii@postgresql.org>; pgsql-committers

Michael Paquier wrote:
> On Sun, Apr 12, 2015 at 10:17 AM, Alvaro Herrera wrote:
> > What pg_basebackup's progress_report() does is have the message in the
> > translatable part not include the \r; the \r is in a separate fprintf()
> > call.
> 
> Like the attached then.

Not a fan of this approach, because now this function knows that
pg_log(PG_PROGRESS) is equivalent to printf().  This abstraction is a
bit leaky, isn't it ...  Probably not worth sweating about, though.

> diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
> index aba12d8..3e2dc76 100644
> --- a/src/bin/pg_rewind/logging.c
> +++ b/src/bin/pg_rewind/logging.c
> @@ -134,7 +134,8 @@ progress_report(bool force)
>  	snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
>  			 fetch_size / 1024);
>  
> -	pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied\r",
> +	pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied",
>  		   (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
>  		   percent);
> +	printf("\r");
>  }



-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-15 03:41  Fujii Masao <masao.fujii@gmail.com>
  parent: Alvaro Herrera <alvherre@2ndquadrant.com>
  0 siblings, 1 reply; 11+ messages in thread

From: Fujii Masao @ 2015-04-15 03:41 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Michael Paquier <michael.paquier@gmail.com>; Peter Eisentraut <peter_e@gmx.net>; Fujii Masao <fujii@postgresql.org>; pgsql-committers

On Tue, Apr 14, 2015 at 2:17 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Michael Paquier wrote:
>> On Sun, Apr 12, 2015 at 10:17 AM, Alvaro Herrera wrote:
>> > What pg_basebackup's progress_report() does is have the message in the
>> > translatable part not include the \r; the \r is in a separate fprintf()
>> > call.
>>
>> Like the attached then.
>
> Not a fan of this approach, because now this function knows that
> pg_log(PG_PROGRESS) is equivalent to printf().  This abstraction is a
> bit leaky, isn't it ...  Probably not worth sweating about, though.
>
>> diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
>> index aba12d8..3e2dc76 100644
>> --- a/src/bin/pg_rewind/logging.c
>> +++ b/src/bin/pg_rewind/logging.c
>> @@ -134,7 +134,8 @@ progress_report(bool force)
>>       snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
>>                        fetch_size / 1024);
>>
>> -     pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied\r",
>> +     pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied",
>>                  (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
>>                  percent);
>> +     printf("\r");
>>  }

So could you elaborate your "favorite" approach?

Now pg_log() calls printf() and fflush(stdout). So '\r' is printed after fflush.
It's a bit strange. Maybe we can just replace pg_log() with printf() here.

Another question is; should we output the progress report to stderr rather
than stdout? I thought this because I found that pg_basebackup reports
the progress to stderr.

Regards,

-- 
Fujii Masao


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-15 07:55  Heikki Linnakangas <hlinnaka@iki.fi>
  parent: Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 1 reply; 11+ messages in thread

From: Heikki Linnakangas @ 2015-04-15 07:55 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Michael Paquier <michael.paquier@gmail.com>; Peter Eisentraut <peter_e@gmx.net>; Fujii Masao <fujii@postgresql.org>; pgsql-committers

On 04/15/2015 06:41 AM, Fujii Masao wrote:
> On Tue, Apr 14, 2015 at 2:17 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Michael Paquier wrote:
>>> On Sun, Apr 12, 2015 at 10:17 AM, Alvaro Herrera wrote:
>>>> What pg_basebackup's progress_report() does is have the message in the
>>>> translatable part not include the \r; the \r is in a separate fprintf()
>>>> call.
>>>
>>> Like the attached then.
>>
>> Not a fan of this approach, because now this function knows that
>> pg_log(PG_PROGRESS) is equivalent to printf().  This abstraction is a
>> bit leaky, isn't it ...  Probably not worth sweating about, though.
>>
>>> diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
>>> index aba12d8..3e2dc76 100644
>>> --- a/src/bin/pg_rewind/logging.c
>>> +++ b/src/bin/pg_rewind/logging.c
>>> @@ -134,7 +134,8 @@ progress_report(bool force)
>>>        snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
>>>                         fetch_size / 1024);
>>>
>>> -     pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied\r",
>>> +     pg_log(PG_PROGRESS, "%*s/%s kB (%d%%) copied",
>>>                   (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
>>>                   percent);
>>> +     printf("\r");
>>>   }
>
> So could you elaborate your "favorite" approach?
>
> Now pg_log() calls printf() and fflush(stdout). So '\r' is printed after fflush.
> It's a bit strange. Maybe we can just replace pg_log() with printf() here.

A better solution from a modularity point of view would be to add a new 
function, pg_progress() for this. It would print the line with pg_log() 
and then print the \r to the end. Then the caller wouldn't need to know 
whether the progress messages are going to stdout, stderr, or somewhere 
else entirely.

> Another question is; should we output the progress report to stderr rather
> than stdout? I thought this because I found that pg_basebackup reports
> the progress to stderr.

Yeah, probably. We should go through all the output and figure out where 
each kind of message needs to do. Should follow the principle Alvaro 
laid out 
(http://www.postgresql.org/message-id/20150407205320.GN4369@alvh.no-ip.org), 
and also make sure it's consistent with pg_basebackup and other tools. 
Michael's patch changed some of the logging but we should take a more 
holistic look at the situation. And add a comment somewhere explaining 
the principle.

- Heikki



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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Mark the second argument of pg_log as the translatable string in
@ 2015-04-15 12:15  Michael Paquier <michael.paquier@gmail.com>
  parent: Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2015-04-15 12:15 UTC (permalink / raw)
  To: hlinnaka@iki.fi; +Cc: Fujii Masao <masao.fujii@gmail.com>; Alvaro Herrera <alvherre@2ndquadrant.com>; Peter Eisentraut <peter_e@gmx.net>; Fujii Masao <fujii@postgresql.org>; pgsql-committers

On Wed, Apr 15, 2015 at 4:55 PM, Heikki Linnakangas wrote:
> On 04/15/2015 06:41 AM, Fujii Masao wrote:
>> Another question is; should we output the progress report to stderr rather
>> than stdout? I thought this because I found that pg_basebackup reports
>> the progress to stderr.
>
>
> Yeah, probably. We should go through all the output and figure out where
> each kind of message needs to do. Should follow the principle Alvaro laid
> out
> (http://www.postgresql.org/message-id/20150407205320.GN4369@alvh.no-ip.org),
> and also make sure it's consistent with pg_basebackup and other tools.
> Michael's patch changed some of the logging but we should take a more
> holistic look at the situation. And add a comment somewhere explaining the
> principle.

Isn't what we are looking for here a common set of frontend-only APIs,
let's say as src/common/logging.c? All the tools we have could use it
without knowing if they output on stdout and stderr.
-- 
Michael


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



^ permalink  raw  reply  [nested|flat] 11+ messages in thread


end of thread, other threads:[~2015-04-15 12:15 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2015-04-08 02:06 pgsql: Mark the second argument of pg_log as the translatable string in Fujii Masao <fujii@postgresql.org>
2015-04-08 04:35 ` Michael Paquier <michael.paquier@gmail.com>
2015-04-08 04:52   ` Fujii Masao <masao.fujii@gmail.com>
2015-04-12 01:08 ` Peter Eisentraut <peter_e@gmx.net>
2015-04-12 01:17   ` Alvaro Herrera <alvherre@2ndquadrant.com>
2015-04-12 06:03     ` Michael Paquier <michael.paquier@gmail.com>
2015-04-13 04:43       ` Fujii Masao <masao.fujii@gmail.com>
2015-04-13 17:17       ` Alvaro Herrera <alvherre@2ndquadrant.com>
2015-04-15 03:41         ` Fujii Masao <masao.fujii@gmail.com>
2015-04-15 07:55           ` Heikki Linnakangas <hlinnaka@iki.fi>
2015-04-15 12:15             ` Michael Paquier <michael.paquier@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox