[Libreoffice] new configmgr access API ...

[Stephan Bergmann]

On 01/13/2012 04:14 PM, Michael Meeks wrote:
> On Fri, 2012-01-13 at 14:15 +0100, Stephan Bergmann wrote:
>> UNO *is* the tool to make functionality available to different
>> languages, to extensions, and to scripting facilities.
>
> 	Sure - of course. This API, on the other hand, is a C++ syntactic
> sugar, so surely we can do things better in our native language
> perhaps ? at least I think taking an efficiency hit to allow the
> possibility of a later non-C++ configmgr implementation is unlikely to
> pay dividends.

For me, its more of an "avoid an additional interface" than "fail to 
improve efficiency of C++ code" thing.  Also, its not entirely a C++ 
coating only, parts of it can benefit other UNO bindings too (and at 
least in principle the type safe wrappings around the configuration 
items could also be moved to plain UNOIDL).

> 	I'd not seen any of this, so I'm just looking for the first time; I
> have a number of queries (given that this will/should be used
> everywhere).
>
> * reading:
> 	workdir/unxlngi6.pro/CustomTarget/officecfg/registry/Office/Common.hxx
>
> 	I love the:
> 	namespace officecfg { namespace Office { namespace Common {
>
> 	ie. we skipped the org and the OpenOffice - which is cool; good
> 	to get over the over-namespacing legacy. My question would be -
> 	do we even need the 'Office' ? ;-) but ... it's prolly fine for
> 	now.

It strictly skips only the never-varying prefix "org/openoffice" (there 
are components like "Inet" and "Setup" next to "Office").

> * string construction:
>
> 	struct UseOldExport: public unotools::ConfigurationProperty<UseOldExport, bool>  {
> 	    static rtl::OUString path() { return rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("/org.openoffice.Office.Common/InternalMSExport/UseOldExport")); }
>
> 	Since we're going to get a lot of these inserted, it might be
> 	rather a good plan to split a path() method from a key()
> 	method so the compiler can share the:
> 		/org.openoffice.Office.Common/.../
> 	for all the keys; that makes the call site slightly larger, but
> 	the resulting binary potentially rather smaller :-)
>
> 	it'd of course also be rather nice to keep the strings
> 	as const char *'s until we get them into some non-inlined code
> 	hiding that construction stuff inside some
> 	static ConfigurationWrapper::getPropertyValue(...) type method ?
> 	That way we get a single string (buffer) construction to build
> 	the path instead of one per call-site.

There's room for improvement, for sure.  I'd save that for when larger 
chunks of client code have been adapted to actually use it.  (The nice 
thing is that optimizations like the above can be introduced completely 
transparently and without the need to worry about any compatibility.)
>
> * XComponentContext-age ...
> 	    static ConfigurationWrapper const&  get(
> 	        com::sun::star::uno::Reference<  com::sun::star::uno::XComponentContext>
> 	const&  context);
>
> 	Do we really need to pass this parameter to these methods ?
>
> 	We have a single configmr instance, it seems unlikely that we
> 	really need anything to help us find it surely ? either
> 	configmgr is there, or the world goes bang :-)
>
> 	If we're desparate to have it (it'd be nice to know what for),
> 	then we could we have a default-to-NULL pointer to a reference
> 	there ?

The UNO concept is to thread the (one and only) component context 
instance through the code, and in this world view 
comphelper::getProcessComponentContext is considered a hack to be 
cleaned up.  Which one is more practical is surely debatable.  (For the 
case at hand, calling comphelper::getProcessComponentContext internally 
in get() rather than passing in XComponentContext would evidently be 
less typing at the typical client side -- but then again, think about 
unit tests and the virtues of reducing global variables.)  I did 
hesitate before making the parameter explicit, but then decided "in 
dubio for clean concepts."

> 	Anyhow - we should clearly create an EasyHack to drive this goodness
> all through the code-base, I suspect it's highly susceptible to
> wide-spread volunteer effort. Are you happy enough with it yet to do
> that ?

As I already wrote: "I haven't announced this new C++ API yet, as some 
issues about change-notification are not yet completely thought out for 
it.  Shame on me, should really do that soon."

Stephan