Re: [lmi] xmlwrapp '-Wconversion' warnings

[Greg Chicares]

On 2019-04-01 18:39, Vadim Zeitlin wrote:
> On Mon, 1 Apr 2019 16:14:38 +0000 Greg Chicares <address@hidden> wrote:
> GC> On 2019-03-27 10:23, Vadim Zeitlin wrote:

[...writing empty elements as "<gloss></gloss>" vs. "<gloss/>"...]

> GC> I've always preferred the compact form because of terseness,
> 
>  Me too, even though I know that the canonical form is the full one.
> 
> GC> Is it definitely your intention in xmlwrapp that set_text_content() prefer
> GC> the compact form despite its being noncanonical?
> 
>  As usual, xmlwrapp doesn't do anything non-trivial here, all it does it
> call libxml2 functions and writing XML out is implemented entirely in the
> latter library and it happens to prefer the compact form by default as

That's good enough for me, then--libxml2 casts the deciding vote:
  +1 terseness
  -1 non-canonical-ness
  +1 libxml2 does <gloss/> by default

> well. However it does provide XML_SAVE_NO_EMPTY flag which can be used to
> avoid creating the compact empty tags. Using it is not completely trivial

>From a high-level-language perspective, nothing libxml2 does is trivial.

> as flags can only be specified using lower-level API and not the high-level
> functions we currently use, but it should definitely be possible, so I
> could look into doing this if you think it would be worth adding the
> possibility to ensure that we never get any non-canonical empty tags in the
> XML output.

Not worth it.

> GC> > xml_serialize_test still passes (and, to be complete, it does not
> GC> > pass, as expected and indicated by the comment, if I use just
> GC> > set_content() above instead)
> GC> 
> GC> lmi's 'xml_serialize_test.cpp' contains:
> GC>   std::string      const s0("string with ampersand & embedded spaces");
> GC> but AFAICS all tests pass both with and without the patch above.
> GC> What am I missing?
> 
>  Maybe nothing and I just didn't explain myself correctly: the test does
> pass both with the original version and the new one, however it does not
> pass if I use the commented out version shown in the old TODO comment, i.e.
> 
>       e.set_content(value_cast<std::string>(t).c_str());
> 
> does not escape the ampersands, while set_text_content() does.

Thanks, now I see:

GC> > -        e.clear();
GC> > -        // XMLWRAPP !! Someday, this might be rewritten thus:
GC> > -        //   e.set_content(value_cast<std::string>(t).c_str());
GC> > -        // but for now that doesn't work with embedded ampersands.
GC> > -        
e.push_back(xml::node(xml::node::text(value_cast<std::string>(t).c_str())));
GC> > +        e.set_text_content(value_cast<std::string>(t).c_str());

I was sure you didn't mean *that* comment because the commented-out
version is obviously exactly the same as the one you added...

GC> > -        //   e.set_content(value_cast<std::string>(t).c_str());
GC> > +        e.set_text_content(value_cast<std::string>(t).c_str());

...except (as I now see by juxtaposing them) where they actually differ.