Fixed redundant i18n strings in json
Please find attached a small tweak to a couple of strings found while
updating translations. The %d version is already used elsewhere in the
same file.
-- Daniele
Attachments:
fixarg.patchtext/x-patch; charset=US-ASCII; name=fixarg.patchDownload
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index 2f99908..2aa27cc 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -1910,7 +1910,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg 1: could not determine data type")));
+ errmsg("arg %d: could not determine data type", 1)));
add_json(arg, false, state, val_type, true);
@@ -1934,7 +1934,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg 2: could not determine data type")));
+ errmsg("arg %d: could not determine data type", 2)));
add_json(arg, PG_ARGISNULL(2), state, val_type, false);
On Wed, Jul 30, 2014 at 2:59 PM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:
Please find attached a small tweak to a couple of strings found while
updating translations. The %d version is already used elsewhere in the
same file.
Probably not a bad idea, but aren't these strings pretty awful anyway?
At a minimum, "arg" as an abbreviation for "argument" doesn't seem
good.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I'd definitely replace /arg/argument/. Furthermore I'd avoid the form
"argument 1: something is wrong": the string is likely to end up in
sentences with other colons so I'd rather use "something is wrong at
argument 1".
Is the patch attached better?
Cheers,
-- Daniele
Show quoted text
On Thu, Jul 31, 2014 at 1:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jul 30, 2014 at 2:59 PM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:Please find attached a small tweak to a couple of strings found while
updating translations. The %d version is already used elsewhere in the
same file.Probably not a bad idea, but aren't these strings pretty awful anyway?
At a minimum, "arg" as an abbreviation for "argument" doesn't seem
good.--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
fixarg2.patchtext/x-patch; charset=US-ASCII; name=fixarg2.patchDownload
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index 2f99908..0c55e9a 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -1910,7 +1910,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg 1: could not determine data type")));
+ errmsg("could not determine data type for argument %d", 1)));
add_json(arg, false, state, val_type, true);
@@ -1934,7 +1934,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg 2: could not determine data type")));
+ errmsg("could not determine data type for argument %d", 2)));
add_json(arg, PG_ARGISNULL(2), state, val_type, false);
@@ -1980,7 +1980,8 @@ json_build_object(PG_FUNCTION_ARGS)
if (nargs % 2 != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid number or arguments: object must be matched key value pairs")));
+ errmsg("invalid number or arguments"),
+ errhint("Object must be matched key value pairs.")));
result = makeStringInfo();
@@ -1994,7 +1995,8 @@ json_build_object(PG_FUNCTION_ARGS)
if (PG_ARGISNULL(i))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg %d: key cannot be null", i + 1)));
+ errmsg("argument %d cannot be null", i + 1),
+ errhint("Object keys should be text.")));
val_type = get_fn_expr_argtype(fcinfo->flinfo, i);
/*
@@ -2016,7 +2018,8 @@ json_build_object(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg %d: could not determine data type", i + 1)));
+ errmsg("could not determine data type for argument %d",
+ i + 1)));
appendStringInfoString(result, sep);
sep = ", ";
add_json(arg, false, result, val_type, true);
@@ -2042,7 +2045,8 @@ json_build_object(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg %d: could not determine data type", i + 2)));
+ errmsg("could not determine data type for argument %d",
+ i + 2)));
add_json(arg, PG_ARGISNULL(i + 1), result, val_type, false);
}
@@ -2099,7 +2103,8 @@ json_build_array(PG_FUNCTION_ARGS)
if (val_type == InvalidOid || val_type == UNKNOWNOID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("arg %d: could not determine data type", i + 1)));
+ errmsg("could not determine data type for argument %d",
+ i + 1)));
appendStringInfoString(result, sep);
sep = ", ";
add_json(arg, PG_ARGISNULL(i), result, val_type, false);
On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:
I'd definitely replace /arg/argument/. Furthermore I'd avoid the form
"argument 1: something is wrong": the string is likely to end up in
sentences with other colons so I'd rather use "something is wrong at
argument 1".Is the patch attached better?
Looks OK to me. I thought someone else might comment, but since no
one has, committed.
(As a note for the future, you forgot to update the regression test
outputs; I took care of that before committing.)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Aug 5, 2014 at 9:35 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:I'd definitely replace /arg/argument/. Furthermore I'd avoid the form
"argument 1: something is wrong": the string is likely to end up in
sentences with other colons so I'd rather use "something is wrong at
argument 1".Is the patch attached better?
Looks OK to me. I thought someone else might comment, but since no
one has, committed.(As a note for the future, you forgot to update the regression test
outputs; I took care of that before committing.)
I think you missed one of the regression tests, see attached
Thanks,
Jeff
Attachments:
regression.diffsapplication/octet-stream; name=regression.diffsDownload
*** /home/jjanes/pgsql/git/src/test/regress/expected/json_1.out Tue Aug 5 14:54:39 2014
--- /home/jjanes/pgsql/git/src/test/regress/results/json.out Tue Aug 5 14:59:29 2014
***************
*** 1146,1152 ****
-- keys must be scalar and not null
SELECT json_build_object(null,2);
! ERROR: arg 1: key cannot be null
SELECT json_build_object(r,2) FROM (SELECT 1 AS a, 2 AS b) r;
ERROR: key value must be scalar, not array, composite, or json
SELECT json_build_object(json '{"a":1,"b":2}', 3);
--- 1146,1153 ----
-- keys must be scalar and not null
SELECT json_build_object(null,2);
! ERROR: argument 1 cannot be null
! HINT: Object keys should be text.
SELECT json_build_object(r,2) FROM (SELECT 1 AS a, 2 AS b) r;
ERROR: key value must be scalar, not array, composite, or json
SELECT json_build_object(json '{"a":1,"b":2}', 3);
======================================================================
On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
I think you missed one of the regression tests, see attached
Woops. Thanks, committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Aug 6, 2014 at 8:35 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
I think you missed one of the regression tests, see attached
Woops. Thanks, committed.
Thanks.
It needs to go into 9_4_STABLE as well.
Cheers,
Jeff
On Thu, Aug 7, 2014 at 8:20 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
It needs to go into 9_4_STABLE as well.
It is worth noticing that the buildfarm is completely in red because
this patch was not backpatched to REL9_4_STABLE:
http://buildfarm.postgresql.org/cgi-bin/show_status.pl
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Aug 6, 2014 at 7:20 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Wed, Aug 6, 2014 at 8:35 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
I think you missed one of the regression tests, see attached
Woops. Thanks, committed.
Thanks.
It needs to go into 9_4_STABLE as well.
Sheesh, where is my brain? Done, thanks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
<daniele.varrazzo@gmail.com> wrote:I'd definitely replace /arg/argument/. Furthermore I'd avoid the form
"argument 1: something is wrong": the string is likely to end up in
sentences with other colons so I'd rather use "something is wrong at
argument 1".Is the patch attached better?
Looks OK to me. I thought someone else might comment, but since no
one has, committed.
It looks to me like this is still wrong:
if (nargs % 2 != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid number or arguments: object must be matched key value pairs")));
+ errmsg("invalid number or arguments"),
+ errhint("Object must be matched key value pairs.")));Surely that was meant to read "invalid number OF arguments". The errhint
is only charitably described as English, as well. I'd suggest something
like "Arguments of json_build_object() must be pairs of keys and values."
--- but maybe someone else can phrase that better.regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane-2 wrote
Robert Haas <
robertmhaas@
> writes:
On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
<
daniele.varrazzo@
> wrote:
I'd definitely replace /arg/argument/. Furthermore I'd avoid the form
"argument 1: something is wrong": the string is likely to end up in
sentences with other colons so I'd rather use "something is wrong at
argument 1".Is the patch attached better?
Looks OK to me. I thought someone else might comment, but since no
one has, committed.It looks to me like this is still wrong:
if (nargs % 2 != 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("invalid number or arguments: object must be matched key value pairs"))); + errmsg("invalid number or arguments"), + errhint("Object must be matched key value pairs.")));Surely that was meant to read "invalid number OF arguments". The errhint is only charitably described as English, as well. I'd suggest something like "Arguments of json_build_object() must be pairs of keys and values." --- but maybe someone else can phrase that better.
The user documentation is worth emulating here:
http://www.postgresql.org/docs/9.4/interactive/functions-json.html
errmsg("argument count must be divisible by 2")
errhint("The argument list consists of alternating names and values")
Note that I s/keys/names/ to match said documentation.
David J.
--
View this message in context: http://postgresql.1045698.n5.nabble.com/Fixed-redundant-i18n-strings-in-json-tp5813330p5814122.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
David G Johnston <david.g.johnston@gmail.com> writes:
Tom Lane-2 wrote
Surely that was meant to read "invalid number OF arguments". The errhint is only charitably described as English, as well. I'd suggest something like "Arguments of json_build_object() must be pairs of keys and values." --- but maybe someone else can phrase that better.
The user documentation is worth emulating here:
http://www.postgresql.org/docs/9.4/interactive/functions-json.html
errmsg("argument count must be divisible by 2")
errhint("The argument list consists of alternating names and values")
Seems reasonable to me.
Note that I s/keys/names/ to match said documentation.
Hm. The docs aren't too consistent either: there are several other nearby
places that say "keys". Notably, the functions json[b]_object_keys() have
that usage embedded in their names, where we can't readily change it.
I'm inclined to think we should s/names/keys/ in the docs instead.
Thoughts?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Aug 7, 2014 at 5:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David G Johnston <david.g.johnston@gmail.com> writes:
Tom Lane-2 wrote
Surely that was meant to read "invalid number OF arguments". The
errhint
is only charitably described as English, as well. I'd suggest something
like "Arguments of json_build_object() must be pairs of keys andvalues."
--- but maybe someone else can phrase that better.The user documentation is worth emulating here:
http://www.postgresql.org/docs/9.4/interactive/functions-json.htmlerrmsg("argument count must be divisible by 2")
errhint("The argument list consists of alternating names and values")Seems reasonable to me.
Note that I s/keys/names/ to match said documentation.
Hm. The docs aren't too consistent either: there are several other nearby
places that say "keys". Notably, the functions json[b]_object_keys() have
that usage embedded in their names, where we can't readily change it.I'm inclined to think we should s/names/keys/ in the docs instead.
Thoughts?
Agreed - have the docs match the common API term usage in our
implementation.
Not sure its worth a thorough hunt but at least fix them as they are
noticed.
David J.