Thread: create_help.pl treats as replaceable
I found it annoying that sql_help.c contains a literal parameter as a translatable string. The cause is that create_help.pl treats <literal>match</> as a replaceable. The attached excludes literals from translatable strings. By a quick look it seems to me that the "match" in "COPY.. HEADER match" is the first and only instance of a literal parameter as of PG15. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Attachment
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> I found it annoying that sql_help.c contains a literal parameter as a
> translatable string.
> The cause is that create_help.pl treats <literal>match</> as a
> replaceable. The attached excludes literals from translatable strings.
> By a quick look it seems to me that the "match" in "COPY.. HEADER
> match" is the first and only instance of a literal parameter as of
> PG15.
Isn't that a documentation bug rather than a problem with create_help?
I see what you're talking about:
HEADER [ <replaceable class="parameter">boolean</replaceable> | <literal>match</literal> ]
but that just seems flat-out wrong. If "match" is a keyword it should
be rendered like other keywords. I'm not very interested in splitting
hairs about whether the grammar thinks it is a keyword --- it looks like
one to a user. So I think
HEADER [ <replaceable class="parameter">boolean</replaceable> | MATCH ]
would be a better solution.
regards, tom lane
At Tue, 17 May 2022 11:09:23 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in > but that just seems flat-out wrong. If "match" is a keyword it should > be rendered like other keywords. I'm not very interested in splitting > hairs about whether the grammar thinks it is a keyword --- it looks like > one to a user. So I think > > HEADER [ <replaceable class="parameter">boolean</replaceable> | MATCH ] > > would be a better solution. Oh, agreed. Thanks for the correction. By the way the error message in defGetCopyHeaderChoice is as follows. "%s requires a Boolean value or \"match\"" Should it be "%s requires a boolean value or MATCH"? At least I think "Boolean" should be un-capitalized. The second attached replaces "Booean" with "boolean" and the \"match\" above to MATCH. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Attachment
On 17.05.22 17:09, Tom Lane wrote: >> By a quick look it seems to me that the "match" in "COPY.. HEADER >> match" is the first and only instance of a literal parameter as of >> PG15. > Isn't that a documentation bug rather than a problem with create_help? Yeah, there is no need for a <literal> inside a <synopsis>. So I just removed it.
On 18.05.22 02:58, Kyotaro Horiguchi wrote:
> Oh, agreed. Thanks for the correction. By the way the error message in
> defGetCopyHeaderChoice is as follows.
>
> "%s requires a Boolean value or \"match\""
>
> Should it be "%s requires a boolean value or MATCH"?
The documentation of COPY currently appears to use the capitalization
OPTION value
so I left it lower-case.
> At least I think "Boolean" should be un-capitalized.
"Boolean" is correct; see <https://en.wiktionary.org/wiki/Boolean> for
example.
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 17.05.22 17:09, Tom Lane wrote:
>> Isn't that a documentation bug rather than a problem with create_help?
> Yeah, there is no need for a <literal> inside a <synopsis>. So I just
> removed it.
I think you should have upper-cased MATCH while at it, to make it clear
that it acts like a keyword in this context. The current situation is
quite unreadable in plain-ASCII output:
regression=# \help copy
Command: COPY
...
HEADER [ boolean | match ]
...
Since "boolean" is a metasyntactic variable here, it's absolutely
not obvious that "match" isn't.
regards, tom lane
At Wed, 18 May 2022 18:23:57 +0200, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in > "Boolean" is correct; see <https://en.wiktionary.org/wiki/Boolean> for > example. Ok, so, don't we in turn need to replace "boolean"s with "Boolean"? "only boolean operators can have negators" "only boolean operators can have restriction selectivity" ... And I'm not sure how to do with "bool". Should it be "Boolean" instead from the point of uniformity? errmsg("only bool, numeric, and text types could be " regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On 18.05.22 18:29, Tom Lane wrote: > I think you should have upper-cased MATCH while at it, to make it clear > that it acts like a keyword in this context. The current situation is > quite unreadable in plain-ASCII output: > > regression=# \help copy > Command: COPY > ... > HEADER [ boolean | match ] > ... > > Since "boolean" is a metasyntactic variable here, it's absolutely > not obvious that "match" isn't. done
On 19.05.22 04:12, Kyotaro Horiguchi wrote: > At Wed, 18 May 2022 18:23:57 +0200, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote in >> "Boolean" is correct; see <https://en.wiktionary.org/wiki/Boolean> for >> example. > > Ok, so, don't we in turn need to replace "boolean"s with "Boolean"? > > "only boolean operators can have negators" > "only boolean operators can have restriction selectivity" > ... > > And I'm not sure how to do with "bool". Should it be "Boolean" instead > from the point of uniformity? > > errmsg("only bool, numeric, and text types could be " The SQL data type is called BOOLEAN, and we typically lower-case type names in PostgreSQL, so messages should be like column %s should be of type integer column %s should be of type boolean As an adjective, not a type, it should be spelled Boolean, because that's how it's in the dictionary (cf. Gaussian). %s should have a string value %s should have a Boolean value "bool" should normally not appear in user-facing messages, unless we are dealing with internal type names (cf. int4) or C types. Of course, the lines between all of the above are blurry.