[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] maint: use common xml quoting style



On 12/07/2011 08:10 PM, Daniel Veillard wrote:
> On Mon, Dec 05, 2011 at 02:48:30PM -0700, Eric Blake wrote:
>> According to the official XML specification [1], attributes
>> can be specified with either ' or " (where the difference is
>> that you can use '"' or '"' but must use """,
>> and conversely for "'" or "'" vs. ''').  But our
>> code generation in src/conf prefers to output the '' notation,
>> as it is easier to write C string literals for that style.
>> Using a consistent style throughout libvirt will make it
>> easier for users to copy-and-paste without wondering why we
>> switch quoting styles mid-stream.
> [...]
>> diff --git a/tests/xml2sexprdata/xml2sexpr-escape.xml b/tests/xml2sexprdata/xml2sexpr-escape.xml
>> index 6eed578..bca2c81 100644
>> --- a/tests/xml2sexprdata/xml2sexpr-escape.xml
>> +++ b/tests/xml2sexprdata/xml2sexpr-escape.xml
>> @@ -5,7 +5,7 @@
>>      <type>hvm</type>
>>      <kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel>
>>      <initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd>
>> -    <cmdline> method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os&amp;version="devel";  </cmdline>
>> +    <cmdline> method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os&amp;version=&quot;devel";  </cmdline>
>>      <loader>/usr/lib/xen/boot/hvmloader</loader>
> 
>   Actually I dislike for this kind of things. It's semantically
> equivalent, and I don't think we should try to enforce arbitrary
> requirements which are not needed. This is actually more confusing
> for people who do know the XML spec,
>   "why isn't my example good"
> it actually is! The choice of quotes is on purpose to allow easy
> serialization when you have ' or " in an attribute value. Requiring
> the user to escape is kind of wrong IMHO.

But the fact that we _didn't_ have any ' or " in attribute values in all
of the conversion is a testament to the fact that we don't want to
choose attributes with quoting in the attribute value.

> Also the automatic rule won't be able to distinguish " and " within
> attribute text content from those within element text content, and
> we certainly don't want to make a simili parser in perl or regexps.
> Also if someone uses an XML editor on one of our fragment it's very
> likely that the ' will be turned into " because the distinction is not
> part of the XML infoset (i.e. the amount of data an XML parser should
> report to the application) and the tool will serialize back using
> his own preferences, and that's just fine.
> 
>   I think the inconvenients in that case are not worth the possible
> improvement.

One of the improvements, which I already have noticed since I wrote the
patch, is that it is easier to search the rng files for a particular
macro definition.  That is, if you KNOW that a syntax-check rule
enforced the style of <define name='blah'>...</define>, it is easier to
see <ref name='blah'/> and search for the definition; whereas pre-patch,
you could see <ref name='blah'/> but not know whether to search for
name='blah' or name="blah" to find the counterpart <define>.

Notice that my syntax-check is not documenting anything contrary to the
user; that is, no where did I change documentation to suggest to the
user that they must prefer '' over "".  Rather it is just presenting a
consistent interface (since 'virsh dumpxml dom' always outputs
name='value', then the web pages that document how to interpret 'virsh
dumpxml' output should also use name='value').  We could drop the
changes to the tests directory, if you'd like (that is, use just patches
2 and 3 for the ease of rng searching and for the consistent
documentation, but leaving the test directory as a way to prove that the
quoting style doesn't matter).

But if you're dead set against this series, I guess it won't hurt things
to drop it (I can continue to do a regex search for name=['"]blah when
looking for a <def name='blah'> counterpart in rng, which is
particularly hard to type at a shell command line, but oh well).  Any
other opinions?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]