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

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

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

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

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

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

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

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

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

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.

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