Обсуждение: Some messages of pg_rewind --debug not getting translated

Hi all,

I found that some of the messages generated by pg_rewind --debug are
not getting translated because the string messages are not put within
_(%s). For the debug messages of the file map and the data page map
this is not really a problem because those do not involve any
translatable words like "blk", but for the timeline history file I
think that's a bug. Attached is a patch putting everything under
pg_log(PG_DEBUG) so as we do not rely directly on printf().
Regards,
--
Michael

Seems reasonable.  For the last hunk in your patch, though, I would add
a /* translator: */ comment explaining what each of the values is;
otherwise it's just incomprehensible percent-sign-salad for the poor
translator.  Note that if the line is too long you have to use dirty
tricks to avoid pgindent from messing it up (see 673b52753489 for
example)

I'm not sure if we should keep "blk" in the original or make it a
complete word.

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

On Wed, Mar 23, 2016 at 12:24 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Seems reasonable.  For the last hunk in your patch, though, I would add
> a /* translator: */ comment explaining what each of the values is;
> otherwise it's just incomprehensible percent-sign-salad for the poor
> translator.  Note that if the line is too long you have to use dirty
> tricks to avoid pgindent from messing it up (see 673b52753489 for
> example)

OK, done this way.

> I'm not sure if we should keep "blk" in the original or make it a
> complete word.

OK, a complete word makes more sense.
--
Michael

Michael Paquier wrote:
> On Wed, Mar 23, 2016 at 12:24 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
> > Seems reasonable.  For the last hunk in your patch, though, I would add
> > a /* translator: */ comment explaining what each of the values is;
> > otherwise it's just incomprehensible percent-sign-salad for the poor
> > translator.  Note that if the line is too long you have to use dirty
> > tricks to avoid pgindent from messing it up (see 673b52753489 for
> > example)
> 
> OK, done this way.
> 
> > I'm not sure if we should keep "blk" in the original or make it a
> > complete word.
> 
> OK, a complete word makes more sense.

Thanks, pushed.

If you're interesting in improving translatability of this program
further, I suggest that messages of this sort msgid "BKPBLOCK_HAS_DATA set, but no data included at %X/%X"
should have a %s instead of the flag name.

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

On Tue, Mar 29, 2016 at 2:45 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Michael Paquier wrote:
>> On Wed, Mar 23, 2016 at 12:24 AM, Alvaro Herrera
>> <alvherre@2ndquadrant.com> wrote:
>> > Seems reasonable.  For the last hunk in your patch, though, I would add
>> > a /* translator: */ comment explaining what each of the values is;
>> > otherwise it's just incomprehensible percent-sign-salad for the poor
>> > translator.  Note that if the line is too long you have to use dirty
>> > tricks to avoid pgindent from messing it up (see 673b52753489 for
>> > example)
>>
>> OK, done this way.
>>
>> > I'm not sure if we should keep "blk" in the original or make it a
>> > complete word.
>>
>> OK, a complete word makes more sense.
>
> Thanks, pushed.

Thanks.

> If you're interesting in improving translatability of this program
> further, I suggest that messages of this sort
>   msgid "BKPBLOCK_HAS_DATA set, but no data included at %X/%X"
> should have a %s instead of the flag name.

I'll think about it. Though I am afraid this would reduce the code
readability in xlogreader.c
-- 
Michael

Michael Paquier wrote:
> On Tue, Mar 29, 2016 at 2:45 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:

> > If you're interesting in improving translatability of this program
> > further, I suggest that messages of this sort
> >   msgid "BKPBLOCK_HAS_DATA set, but no data included at %X/%X"
> > should have a %s instead of the flag name.
> 
> I'll think about it. Though I am afraid this would reduce the code
> readability in xlogreader.c

Oh, I didn't notice these came from xlogreader.  Hmm, but isn't it just
a change like this?

*** a/src/backend/access/transam/xlogreader.c
--- b/src/backend/access/transam/xlogreader.c
***************
*** 1043,1050 **** DecodeXLogRecord(XLogReaderState *state, XLogRecord *record, char **errormsg)             if
(blk->has_data&& blk->data_len == 0)             {                 report_invalid_record(state,
 
!                       "BKPBLOCK_HAS_DATA set, but no data included at %X/%X",
!                                       (uint32) (state->ReadRecPtr >> 32), (uint32) state->ReadRecPtr);
goto err;             }             if (!blk->has_data && blk->data_len != 0)
 
--- 1043,1052 ----             if (blk->has_data && blk->data_len == 0)             {
report_invalid_record(state,
!                                       "%s set, but no data included at %X/%X",
!                                       "BKPBLOCK_HAS_DATA",
!                                       (uint32) (state->ReadRecPtr >> 32),
!                                       (uint32) state->ReadRecPtr);                 goto err;             }
if (!blk->has_data && blk->data_len != 0)
 

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