RE: [libxml++] keek blancs.

[andy glew amd com]

> Not keeping blancs at parsing time or adding some when 
> writing to indent is breaking the XML specifications.
> However I think it can be useful to do so.

I agree that it is useful to do so
- sufficiently useful that some such functionality
should be provided as part of a standard library
like libxml++ (or else it will be constantly reinvented
in slightly different ways).


> So I propose to modify the API of libxml to give the 
> possibility not to keep blanks (xmlKeepBlanksDefault option in libxml).

However, I question whether it is appropriate to add 
options to the parsing functions.  That's a slippery 
slope.  If more options get added this way it leads to
"fragile base class", which matters to me because I hope/plan 
to have my own Parser derived class, something like TwigParser
(halfway between DomParser and SaxParser).



Better to make it a property... since C++ doesn't really have
properties, make it a data member of Parser, with the appropriate
accessors.

>  1) add a parameter to parsing functions :
> Document* 
>     xmlpp::parse_file(
>         const std::string& filename,
>         bool keepblanks = KeepBlancs::Default
>     )
>     throw(exception)
> etc.

Q: I'm a little bit confused by this.
I thought that parse_file, etc., were methods on
a parser object, e.g. 
xmlpp::Parser::parse_file,
xmlpp::DomParser::parse_file,
xmlpp::SaxParser::parse_file.

Has it been made into a free function in a version
I have not downloaded yet?
(I wish I could get CVS across my company's firewall,
legitimately.)

Anyway, assuming that it has not been:

Make your whitespace handling mode a private data member
of Parser.  With the appropriate accessors.

>  2) add an accessor to Parser :
> Parser::set_keepblanks(bool value)

A slight trick, from Stroustrup, that gives you the equivalent 
of keyword options:

Instead of 
  void Parser::set_keepblanks(bool value)
make it
  Parser& Parser::set_keepblanks(bool value)
as in
  Parser& Parser::set_keepblanks(bool value) {
      this->m_keepblanks = value;
      return *this;
  }

So, instead of  
  DomParser domparser;
  domparser.parse_file(filename,/*keepblanks=*/true);
you do
  domparser.set_keepblanks(true).parse_file(filename);


Such "Stroustrup keywords" have some minor issues:
either you want to make parse_file a virtual function in Parser,
so that it picks up the derived DomParser::parse_file or 
SaxParser::parse_file; or you have to make set_keepblanks
a delegate in derived classes, with a slightly different
return.  {Virtual functions better, IMHO.}



>  3) add following functions to Document :
> Document::write_to_formated_file(const std::string& filename, const 
> std::string& encoding = std::string()) throw(exception);
> Document::write_to_formated_memory(const std::string& filename, const 
> std::string& encoding = std::string()) throw(exception);

First, spelling: write_to_formatted_file...

Second, this caused me to notice the assymmetry:
   Parser::parse_file(string_filename), 
   Parser::parse_memory(string), 
   Parser::parse_stream(istream)
   Document::write_to_file(string_filename), 
   Document::write_to_string(string)
"Obviously" want write_to_stream(ostream).

It also tends to suggest that what is wanted is a Formatter object,
separate from the Document. Possibly an object that exposes both a
Parser interface for input, and a Formatter interface for output,
since much of the time the input and output keep blanks setting 
will be similar.
   Maybe "Printer" is a betteer name than "Formatter".
What's an antonym for "Parser"?

But that's gilding the lily - BDUF, Big Design Up Front.  
For now, 
	Document::write_to_formatted_file
is fine, with the assunmption being that Document
will have copied the keepblanks setting 
(aka the Formatter object) from the Parser that created it.




> The default behavior would remain the same.
> 
> Do you agree on this ?

Overall, yes, with the minor tweaks to the interface
that I mention above.