create_help.pl treats <literal> 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
Attachments:
0001-Do-not-treat-literal-words-as-translatable.patchtext/x-patch; charset=us-asciiDownload
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
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 thinkHEADER [ <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:
0001-Fix-copy-from-doc.patchtext/x-patch; charset=us-asciiDownload
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
0002-Fix-wording-convention-in-error-messages.patchtext/x-patch; charset=us-asciiDownload
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
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.