Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.

On Wed, Jun 13, 2012 at 12:27 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Jun 13, 2012 at 11:55 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> In any case, the proposed scheme for providing context requires that
>>> you know where the error is before you can identify the context.  I
>>> considered schemes that would keep track of the last N characters or
>>> line breaks in case one of them proved to be the one we need.  That
>>> would add enough cycles to non-error cases to almost certainly not be
>>> desirable.  I also considered trying to back up, but that doesn't look
>>> promising either for arbitrary multibyte encodings.
>
>> Oh, I see.  :-(
>
> Attached is a complete proposed patch for this, with some further
> adjustments to make the output look a bit more like what we use for
> SQL error context printouts (in particular, "..." at both ends of the
> excerpt when appropriate).

I like some of these changes - in particular, the use of errcontext(),
but some of them still seem off.

! DETAIL: Token "'" is invalid.
! CONTEXT: JSON data, line 1: '...

This doesn't make sense to me.

! DETAIL: Character with value 0x0a must be escaped.
! CONTEXT: JSON data, line 1: "abc
! ...

This seems an odd way to present this, especially if the goal is to
NOT include the character needing escaping in the log unescaped, which
I thought was the point of saying 0x0a.

! DETAIL: "\u" must be followed by four hexadecimal digits.
! CONTEXT: JSON data, line 1: "\u000g...

Why does this end in ... even though there's nothing further in the
input string?

> One thing I did not touch, but am less than happy with, is the wording
> of this detail message:
>
>        errdetail("Expected string, number, object, array, true, false, or null, but found \"%s\".",
>                  token),
>
> This seems uselessly verbose to me.  It could be argued that enumerating
> all the options is helpful to rank JSON novices ... but unless you know
> exactly what an "object" is and why that's different from the other
> options, it's not all that helpful.  I'm inclined to think that
> something like this would be better:
>
>        errdetail("Expected JSON value, but found \"%s\".",
>                  token),
>
> Thoughts?

Good idea.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company