pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedcreate_help.pl treats <literal> as replaceable
9+ messages / 3 participants
[nested] [flat]
* create_help.pl treats <literal> as replaceable
@ 2022-05-17 08:43 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Kyotaro Horiguchi @ 2022-05-17 08:43 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
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
Attachments:
[text/x-patch] 0001-Do-not-treat-literal-words-as-translatable.patch (978B, ../../20220517.174342.1884842412165214815.horikyota.ntt@gmail.com/2-0001-Do-not-treat-literal-words-as-translatable.patch)
download | inline diff:
From 81770afd9df6f073c50f875b2dd540273ecc15fa Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 17 May 2022 17:37:33 +0900
Subject: [PATCH] Do not treat <literal> words as translatable
create_help.pl extracts <literal> words as translatable but actually
they are untranslatable. Exclude such words from translatable words
so that translators don't mistakenly translate the words.
---
src/bin/psql/create_help.pl | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/bin/psql/create_help.pl b/src/bin/psql/create_help.pl
index 1a9836cbcc..9919aaf4e7 100644
--- a/src/bin/psql/create_help.pl
+++ b/src/bin/psql/create_help.pl
@@ -138,6 +138,11 @@ foreach my $file (sort readdir DIR)
while ($cmdsynopsis =~ m!<(\w+)[^>]*>(.+?)</\1[^>]*>!)
{
+ if ($1 eq "literal")
+ {
+ $cmdsynopsis =~ s!<(\w+)[^>]*>(.+?)</\1[^>]*>!\2!;
+ next;
+ }
my $match = $2;
$match =~ s/<[^>]+>//g;
$match =~ s/%%/%/g;
--
2.27.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-05-17 15:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 00:58 ` Re: create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-18 16:22 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 2 replies; 9+ messages in thread
From: Tom Lane @ 2022-05-17 15:09 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
@ 2022-05-18 00:58 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-18 16:23 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
1 sibling, 1 reply; 9+ messages in thread
From: Kyotaro Horiguchi @ 2022-05-18 00:58 UTC (permalink / raw)
To: tgl@sss.pgh.pa.us; +Cc: pgsql-hackers@lists.postgresql.org
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
Attachments:
[text/x-patch] 0001-Fix-copy-from-doc.patch (1.3K, ../../20220518.095805.2164390691449974704.horikyota.ntt@gmail.com/2-0001-Fix-copy-from-doc.patch)
download | inline diff:
From 8fc4910b57868f8cb2c68d1af31f3d96523e036b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 18 May 2022 09:43:13 +0900
Subject: [PATCH 1/2] Fix copy-from doc
An option keyword "match" is marked as <literal> but it should be bare
upper-cased word. That mistake caused sql_help.c contain an extra
translatable word.
---
doc/src/sgml/ref/copy.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index e7a6676efd..a3411b4564 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -36,7 +36,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
FREEZE [ <replaceable class="parameter">boolean</replaceable> ]
DELIMITER '<replaceable class="parameter">delimiter_character</replaceable>'
NULL '<replaceable class="parameter">null_string</replaceable>'
- HEADER [ <replaceable class="parameter">boolean</replaceable> | <literal>match</literal> ]
+ HEADER [ <replaceable class="parameter">boolean</replaceable> | MATCH ]
QUOTE '<replaceable class="parameter">quote_character</replaceable>'
ESCAPE '<replaceable class="parameter">escape_character</replaceable>'
FORCE_QUOTE { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
--
2.27.0
[text/x-patch] 0002-Fix-wording-convention-in-error-messages.patch (3.4K, ../../20220518.095805.2164390691449974704.horikyota.ntt@gmail.com/3-0002-Fix-wording-convention-in-error-messages.patch)
download | inline diff:
From 12e321e909f7ca6a20e2a94ccf47fcdaef77a99a Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 18 May 2022 09:50:16 +0900
Subject: [PATCH 2/2] Fix wording convention in error messages
Some error messages wrongly used capitalized "Boolean" in the middle a
sentence. Also fix a use of quoted lower-cased word for a keyword
"MATCH" to be bare, upper-cased word.
---
src/backend/commands/copy.c | 2 +-
src/backend/commands/define.c | 2 +-
src/backend/commands/extension.c | 6 +++---
src/backend/utils/misc/guc.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f448d39c7e..b76c5700ed 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -366,7 +366,7 @@ defGetCopyHeaderChoice(DefElem *def)
}
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("%s requires a Boolean value or \"match\"",
+ errmsg("%s requires a boolean value or MATCH",
def->defname)));
return COPY_HEADER_FALSE; /* keep compiler quiet */
}
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 0755ab1eae..19abb07614 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -151,7 +151,7 @@ defGetBoolean(DefElem *def)
}
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("%s requires a Boolean value",
+ errmsg("%s requires a boolean value",
def->defname)));
return false; /* keep compiler quiet */
}
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 767d9b9619..f95ae4964d 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -560,7 +560,7 @@ parse_extension_control_file(ExtensionControlFile *control,
if (!parse_bool(item->value, &control->relocatable))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value",
+ errmsg("parameter \"%s\" requires a boolean value",
item->name)));
}
else if (strcmp(item->name, "superuser") == 0)
@@ -568,7 +568,7 @@ parse_extension_control_file(ExtensionControlFile *control,
if (!parse_bool(item->value, &control->superuser))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value",
+ errmsg("parameter \"%s\" requires a boolean value",
item->name)));
}
else if (strcmp(item->name, "trusted") == 0)
@@ -576,7 +576,7 @@ parse_extension_control_file(ExtensionControlFile *control,
if (!parse_bool(item->value, &control->trusted))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value",
+ errmsg("parameter \"%s\" requires a boolean value",
item->name)));
}
else if (strcmp(item->name, "encoding") == 0)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 8e9b71375c..0408754e68 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7369,7 +7369,7 @@ parse_and_validate_value(struct config_generic *record,
{
ereport(elevel,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value",
+ errmsg("parameter \"%s\" requires a boolean value",
name)));
return false;
}
--
2.27.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 00:58 ` Re: create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-05-18 16:23 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-19 02:12 ` Re: create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2022-05-18 16:23 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; tgl@sss.pgh.pa.us; +Cc: pgsql-hackers@lists.postgresql.org
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.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 00:58 ` Re: create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-18 16:23 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
@ 2022-05-19 02:12 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-23 11:17 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 1 reply; 9+ messages in thread
From: Kyotaro Horiguchi @ 2022-05-19 02:12 UTC (permalink / raw)
To: peter.eisentraut@enterprisedb.com; +Cc: tgl@sss.pgh.pa.us; pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 00:58 ` Re: create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-18 16:23 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-19 02:12 ` Re: create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-05-23 11:17 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 0 replies; 9+ messages in thread
From: Peter Eisentraut @ 2022-05-23 11:17 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: tgl@sss.pgh.pa.us; pgsql-hackers@lists.postgresql.org
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.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
@ 2022-05-18 16:22 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-18 16:29 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2022-05-18 16:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 16:22 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
@ 2022-05-18 16:29 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-05-23 11:11 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2022-05-18 16:29 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: create_help.pl treats <literal> as replaceable
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 16:22 ` Re: create_help.pl treats <literal> as replaceable Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-18 16:29 ` Re: create_help.pl treats <literal> as replaceable Tom Lane <tgl@sss.pgh.pa.us>
@ 2022-05-23 11:11 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
0 siblings, 0 replies; 9+ messages in thread
From: Peter Eisentraut @ 2022-05-23 11:11 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2022-05-23 11:17 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-05-17 08:43 create_help.pl treats <literal> as replaceable Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 15:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-05-18 00:58 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-18 16:23 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-19 02:12 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-23 11:17 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-18 16:22 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
2022-05-18 16:29 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-05-23 11:11 ` Peter Eisentraut <peter.eisentraut@enterprisedb.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox