Thread: 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
Attachment
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
Attachment
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