Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Sugamoto Shinya <shinya34892(at)gmail(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
Date: 2025-11-19 13:56:59
Message-ID: b57c5768-d117-4e2a-b6cd-edd0e1432d45@eisentraut.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 19.11.25 14:47, Fujii Masao wrote:
> On Wed, Nov 19, 2025 at 10:04 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>
>>> On 19 Nov 2025, at 14:01, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>>> You changed the HINT message to use format arguments,
>>> but I still think it's better to embed the values directly (e.g.,
>>> Valid encodings are "base64", "base64url", "escape", and "hex".)
>>> for consistency with similar HINT messages. If this message is
>>> used in many places a parameterized style would help translators,
>>> but that benefit doesn't apply here.
>>>
>>> Attached is an updated version of the patch. I switched
>>> the HINT message to the embedded form and updated the commit message.
>>> Barring any objections, I'm thinking to commit this.
>>
>> My intepreration was that this version was already objected to by Peter
>> upthread in c5c937dc-de8e-4284-be25-5d5eaf089d00(at)eisentraut(dot)org(dot) The point
>> there being that punctuation in lists is subject to translation.
>
> Yes, so I was thinking a message like "Valid encodings are %s" isn't acceptable
> since the punctuation included in the %s value is not translated. In contrast,
> a message like "Valid encodings are \"base64\", …" is fine because all
> punctuation
> is part of the translatable string itself, I thought.

My point was a different one. It is generally preferable to separate
translatable and untranslated things. So, as an example from elsewhere,
instead of

errhint("Use ALTER DOMAIN instead.");

it would be better to use

errhint("Use % instead.", "ALTER DOMAIN");

Similarly, here I would prefer something along the lines of

errhint("Valid values are \"%s\", \"%s\", and \"%s\".", "foo", "bar",
"baz");

So like it was done in patch v3 looks good to me.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2025-11-19 14:03:03 Re: Trying out <stdatomic.h>
Previous Message Fujii Masao 2025-11-19 13:51:35 Re: regarding statistics retaining with 18 Upgrade