From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Euler Taveira <euler(dot)taveira(at)2ndquadrant(dot)com> |
Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Log message for GSS connection is missing once connection authorization is successful. |
Date: | 2020-11-03 07:18:50 |
Message-ID: | CALDaNm36xidovwu89ELA4fKaHVAocC0-HUtODrohjhfqRk0iJA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Nov 1, 2020 at 3:34 AM Euler Taveira
<euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>
> On Sat, 31 Oct 2020 at 00:34, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>>
>> On Fri, Oct 30, 2020 at 6:35 PM Euler Taveira
>> <euler(dot)taveira(at)2ndquadrant(dot)com> wrote:
>> >
>> > + appendStringInfo(&logmsg, "replication ");
>> > +
>> > + appendStringInfo(&logmsg, "connection authorized: user=%s",
>> > + port->user_name);
>> > + if (!am_walsender)
>> > + appendStringInfo(&logmsg, " database=%s", port->database_name);
>> > +
>> > + if (port->application_name != NULL)
>> > + appendStringInfo(&logmsg, " application_name=%s",
>> > + port->application_name);
>> > +
>> >
>> > Your approach breaks localization. You should use multiple errmsg.
>> >
>>
>> IIUC, isn't it enough calling a single errmsg() inside ereport() with
>> the prepared logmsg.data (which is a string)? The errmsg() function
>> will do the required translation of the logmsg.data. Why do we need
>> multiple errmsg() calls? Could you please elaborate a bit on how the
>> way currently it is done in the patch breaks localization?
>>
>
> No. The strings are specified in the appendStringInfo, hence you should add _()
> around the string to be translated. There is nothing to be translated if you
> specify only the format identifier. You can always test if gettext extracts the
> string to be translated by executing 'make update-po' (after specifying
> --enable-nls in the configure). Search for your string in one of the generated
> files (po/LL.po.new).
>
> You shouldn't split messages like that because not all languages have the same
> order as English. Having said that you risk providing a nonsense translation
> because someone decided to translate pieces of a sentence separately.
>
> + appendStringInfo(&logmsg, "replication ");
> +
> + appendStringInfo(&logmsg, "connection authorized: user=%s",
> + port->user_name);
>
> This hunk will break translation. In Portuguese, the adjective "replication" is
> translated after the noun "connection". If you decided to keep this code as is,
> the printed message won't follow the grammar rules. You will have "replicação
> conexão autorizada" instead of "conexão de replicação autorizada". The former
> isn't grammatically correct. Avoid splitting sentences that are translated.
>
Thanks for the explanation, I have attached a v5 patch with the
changes where the translation should not have any problem.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Improving-the-connection-authorization-message-fo.patch | text/x-patch | 12.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jinbao Chen | 2020-11-03 07:29:38 | Re: Add table AM 'tid_visible' |
Previous Message | Peter Eisentraut | 2020-11-03 07:08:14 | Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path |