On Fri, 2012-01-13 at 14:15 +0100, Stephan Bergmann wrote:
> On 01/13/2012 11:23 AM, Michael Meeks wrote:
> >     Oooh ! :-) it looks really rather nice; how efficient is the compiled
> > representation ? hopefully much more so than the big chunks of in-lined
> > UNO-ness that existing code uses :-)
> 
> It still uses UNO to access configmgr, but through a simplified 
> interface (new com.sun.star.configuration.{ReadOnly,ReadWrite}Access 
> singletons).  The main benefit (besides shorter client code) is type 
> safety

        Yep - it's pretty awesome - it's also far less verbose than many of the
configmgr access sites I've seen:

officecfg::Office::Common::AsianLayout::CompressCharacterDistance::get(comphelper::getProcessComponentContext());

        vs.

        css::uno::Any aVal = ::comphelper::ConfigurationHelper::readDirectKey(
                                xSMGR,
                                
::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("org.openoffice.Office.Common/")),
                                
::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("Misc")),
                                
::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("MaxOpenDocuments")),
                                ::comphelper::ConfigurationHelper::E_READONLY);
        sal_Int32 nCount;
        aVal >>= nCount;
        etc.

        So really nice - and has no visible operator overload too; I love
it :-)

>  -- neither can there be misspellings in the paths of 
> configuration nodes nor confusion in the values that can be read or 
> written for those nodes.

        Sure - and I imagine there is a really serious error if the key is not
there or we get an exception reading it, that we can un-conditionally
dump something nice out for to the console when we get to it :-)

> 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.

        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.

* optimising
        Did I mention that I love the ability to transparently optimise
        this later as a major feature of this :-)

* 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.

* 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 ?


        And that's about it at first glance. Most of these details can be
hammered out later though, so it's mainly the API that's interesting -
so - just the XComponentContext clutter (?).

        Personally, I'd love to see direct linking to configmgr so we don't
need the component context, and also some punch-throughs, so we don't
need the UNO API bits that force us to allocate OUStrings and Any's etc.

        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 ?

        Great work !

                Michael.

-- 
michael.me...@suse.com  <><, Pseudo Engineer, itinerant idiot

_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to