Обсуждение: 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

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

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.