create_help.pl treats <literal> as replaceable

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&gt; 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&gt; 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&gt; 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.