Re: [lmi] Empty xml child text nodes [Was: product editor patch]

[Greg Chicares]

On 2008-03-30 17:17Z, Greg Chicares wrote:
> On 2008-03-28 13:38Z, Vaclav Slavik wrote:
>> Greg Chicares wrote:
> [...]
>>> (D) inputillus_xml_io.cpp, ledger_xml_io.cpp, xml_lmi.cpp
>>> Apparently the purpose of these changes is to avoid writing
>>> empty xml child text nodes. Can you give me an example of the
>>> problem that this change would solve? Is it simply a matter of
>>> writing '<X/>' instead of '<X></X>' because the former seems
>>> more tasteful?
>> 
>> As far as I can tell, yes.
> 
> Let me ask my US coworkers to apply those patch hunks and report
> the results of their testing.

I'm not sure that was ever done. But '<X/>' is better than '<X></X>':
it's more compact, making file operations slightly faster; and writing
xml in the canonical form emitted by xmllint provides a useful test,
which becomes more important as we maintain many xml files by editing
them directly. So I'd like to secure this advantage now. However:

>> (And shorter, too, and add_node() was 
>> already used in other places, so it made sense to use it for 
>> consistency alone when I saw this change.)

xml_lmi::add_node() no longer exists. I tried making changes in
the spirit of the original patch, but they were too pervasive:
I didn't feel certain that I had changed everything I should,
or that I hadn't changed things I shouldn't. Then I thought:

> I wonder whether changing push_back() itself would be better
> (we already modify the library via 'xmlwrapp-0.5.0.patch').

And then I rethought it. The real issue isn't that we add element
nodes whose content is empty; it's that we create such nodes in
the first place. So I'm thinking of applying the 'xmlwrapp' patch
below, which I believe will do exactly what we want; can you see
any reason why that would be a bad idea?

diff --recursive '--unified=3' original/xmlwrapp-0.6.0/src/libxml/node.cxx 
modified/xmlwrapp-0.6.0/src/libxml/node.cxx
--- original/xmlwrapp-0.6.0/src/libxml/node.cxx 2009-02-21 16:19:55.000000000 
+0000
+++ modified/xmlwrapp-0.6.0/src/libxml/node.cxx 2012-02-23 07:26:40.000000000 
+0000
@@ -243,12 +243,14 @@
     pimpl_->xmlnode_ = xmlNewNode(0, reinterpret_cast<const xmlChar*>(name));
     if (!pimpl_->xmlnode_) throw std::bad_alloc();

-    xmlNodePtr content_node = xmlNewText(reinterpret_cast<const 
xmlChar*>(content));
-    if (!content_node) throw std::bad_alloc();
+    if (0 != std::strlen(content)) {
+        xmlNodePtr content_node = xmlNewText(reinterpret_cast<const 
xmlChar*>(content));
+        if (!content_node) throw std::bad_alloc();

-    if (!xmlAddChild(pimpl_->xmlnode_, content_node)) {
-       xmlFreeNode(content_node);
-       throw std::bad_alloc();
+        if (!xmlAddChild(pimpl_->xmlnode_, content_node)) {
+        xmlFreeNode(content_node);
+        throw std::bad_alloc();
+        }
     }

     ap.release();