Re: [boost] [Boost Review] Property Tree Library

From: Marcin Kalicinski (kalita_at_[hidden])
Date: 2006-04-19 09:05:57

> [...] The documentation also requires attention
> from a native English speaker to correct the various grammar errors.

My english is far from perfect. I would be very grateful if a native speaker 
could skim the docs and point me towards several most glaring errors.

> The library seems very well suited to read and save XML configuration
> files. I would suggest, however, that versions of read/write_xml are
> introduced that allow to specify the special keys for attributes,
> comments and text. Also, a flag should be added that specifies
> whitespace handling. (Ignore, preserve, collapse.)

Agreed. XML parser definitely needs more flexibility. All the above 
improvements are easy to implement.

> Additional suggestions:
> 1) Having a separate namespace for the interface of each parser seems to
> be overly verbose. The property_tree namespace is sparsely populated 
> [...]

The namespaces issue has been also mentioned by other people before the 
review. One suggestion was to rename ptree class to property_tree, and place 
it directly in boost namespace.

Current implementation has a virtue of being extremely conservative, trying 
to minimize number of names introduced into existing namespaces.

Because fiddling with namespaces is quite a fundamental change for the 
library interface, it would be nice to gather more suggestions in this 
matter before making any decisions. On the other hand, this is probably 
something that should be addressed before the library becomes a part of 
boost (if ever), because maintaining backwards compatibility with current 
solution would be a madman's nightmare.

> 2) Add a parser for Java-like .properties files. The hierarchy could be
> built using common prefixes, i.e. [...]

I'm not familiar with .properties file format, but it looks easy to 
implement a parser with Spirit. If so, it could be a matter of a day or two 
to have it working.

> 3) The default separator should be settable. E.g. if I want to always
> separate my paths with '/' (I could be porting from a different
> library), I want to be able to write:
> pt.set_default_separator('/');
> string libs = pt.get("build/dirs/libraries");

Agreed. I thought about it before, and came to a conclusion that the best 
way to implement it is to use preprocessor constant to allow user specify 
default separator, e.g. BOOST_PROPERTY_TREE_SEPARATOR.

Thank you,
Marcin