| From: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
|---|---|
| To: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | Nikolay Samokhvalov <samokhvalov(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrey Borodin <amborodin86(at)gmail(dot)com> |
| Subject: | Re: [PATCH] Add pretty-printed XML output option |
| Date: | 2023-02-23 07:51:47 |
| Message-ID: | 3baf8f25-3bd7-7db5-0b31-e4c92b6269ba@enterprisedb.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 23.02.23 07:36, Jim Jones wrote:
> On 23.02.23 02:52, Peter Smith wrote:
>> Here are my review comments for patch v17-0001.
>>
>> ======
>> src/test/regress/sql/xml.sql
>>
>> The blank line(s) which previously separated the xmlserialize tests
>> from the xml IS [NOT] DOCUMENT tests are now missing...
>
> v18 adds a new line in the xml.sql file to separate the xmlserialize
> test cases from the rest.
In kwlist.h you have
PG_KEYWORD("indent", INDENT, UNRESERVED_KEYWORD, AS_LABEL)
but you can actually make it BARE_LABEL, which is preferable.
More importantly, you need to add the new keyword to the
bare_label_keyword production in gram.y. I thought we had some tooling
in the build system to catch this kind of omission, but it's apparently
not working right now.
Elsewhere, let's rename the xmlformat() C function to xmlserialize() (or
maybe something like xmlserialize_indent()), so the association is clearer.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jim Jones | 2023-02-23 08:20:00 | Re: [PATCH] Add pretty-printed XML output option |
| Previous Message | Peter Eisentraut | 2023-02-23 07:35:47 | Make some xlogreader messages more accurate |