Re: [lmi] xmlwrapp '-Wconversion' warnings

[Greg Chicares]

[...having already committed and pushed the patch below, I now want to ask...]

On 2019-03-27 10:23, Vadim Zeitlin wrote:
> On Wed, 27 Mar 2019 00:40:15 +0000 Greg Chicares <address@hidden> wrote:
[...]
> 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())));
> 
>  Hmm, hasn't this been addressed by the addition of set_text_content() back
> in xmlwrapp 0.7.0? I've just applied the following patch:
> 
> ---------------------------------- >8 --------------------------------------
> diff --git a/xml_serialize.hpp b/xml_serialize.hpp
> index 5c6e90156..cef39daff 100644
> --- a/xml_serialize.hpp
> +++ b/xml_serialize.hpp
> @@ -60,11 +60,7 @@ struct xml_io
> 
>      static void to_xml(xml::element& e, T const& t)
>      {
> -        e.clear();
> -        // XMLWRAPP !! Someday, this might be rewritten thus:
> -        //   e.set_content(value_cast<std::string>(t).c_str());
> -        // but for now that doesn't work with embedded ampersands.
> -        
> e.push_back(xml::node(xml::node::text(value_cast<std::string>(t).c_str())));
> +        e.set_text_content(value_cast<std::string>(t).c_str());
>      }
> 
>      static void from_xml(xml::element const& e, T& t)
> ---------------------------------- >8 --------------------------------------

This induces a superficial regression in generated product files, e.g.:

--- data/sample.rounding        2019-01-23 19:22:18.930313781 +0000
+++ /opt/lmi/data/sample.rounding       2019-04-01 15:19:50.547602533 +0000
@@ -5,106 +5,106 @@
   <RoundCoiCharge>
     <decimals>2</decimals>
     <style>To nearest</style>
-    <gloss></gloss>
+    <gloss/>
   </RoundCoiCharge>
...

"<gloss></gloss>" and "<gloss/>" are of course equivalent but not identical.

I've always preferred the compact form because of terseness, and I thought
the compact form was canonical...but I was wrong, as w3.org's c14n would
replace "<gloss/>" with "<gloss></gloss":

  https://www.w3.org/TR/xml-c14n2/#sec-Output-Rules
| Empty elements are converted to start-end tag pairs.

Is it definitely your intention in xmlwrapp that set_text_content() prefer
the compact form despite its being noncanonical? I'm not sure I have an
opinion either way; but this causes regressions in our proprietary
product-files repository, and I don't want to push a commit to adapt its
(version-controlled) generated files to this change until I'm sure you
won't want to change xmlwrapp to make set_text_content()'s output
canonical.

> xml_serialize_test still passes (and, to be complete, it does not pass, as
> expected and indicated by the comment, if I use just set_content() above
> instead)

lmi's 'xml_serialize_test.cpp' contains:
  std::string      const s0("string with ampersand & embedded spaces");
but AFAICS all tests pass both with and without the patch above.
What am I missing?