Re: 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(dot)varrazzo(at)gmail(dot)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(at)gmail(dot)com> wrote:
> On Wed, Jul 30, 2014 at 2:59 PM, Daniele Varrazzo
> <daniele(dot)varrazzo(at)gmail(dot)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(dot)varrazzo(at)gmail(dot)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 9:35 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
> <daniele(dot)varrazzo(at)gmail(dot)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

On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)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 Wed, Aug 6, 2014 at 8:35 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)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(dot)janes(at)gmail(dot)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(dot)janes(at)gmail(dot)com> wrote:
> On Wed, Aug 6, 2014 at 8:35 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>
>> On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)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(at)gmail(dot)com> writes:
> On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
> <daniele(dot)varrazzo(at)gmail(dot)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 &lt;

> robertmhaas@

> &gt; writes:
>> On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo
>> &lt;

> daniele.varrazzo@

> &gt; 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(dot)g(dot)johnston(at)gmail(dot)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

On Thu, Aug 7, 2014 at 5:53 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David G Johnston <david(dot)g(dot)johnston(at)gmail(dot)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?
>
>
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.