Re: [Libreoffice] Simpler logging using a string format function

On Monday 12 of December 2011, Stephan Bergmann wrote:
> On 12/12/2011 04:30 PM, Lubos Lunak wrote:
> >   I'd like to propose changes to the SAL_INFO etc. family of the new
> > logging functions that would replace the somewhat strange usage
> >
> > SAL_INFO("foo", "string "<<  s<<  " of length "<<  n)
> >
> >   with
> >
> > SAL_INFO("foo", "string %1 of length %2", s, n )
> >
> >   while still leaving the possibility to do
> >
> > SAL_INFO("foo", "string " + s + " of length "  + OUString::valueOf( n ))
> >
> >   The last two are IMO much more natural than the iostream-based usage.
>
> Shrug.  Neither << nor something printf-like is particularly sexy, IMO.
>   And trading << for +, when it requires you to wrap non-string
> arguments in rtl::OUString::valueOf, doesn't look too exciting to me,
> either.

 But it might the moment you realize you're trading away things like

 SAL_INFO( "foo", 1 << 2 ) or SAL_INFO( "foo", "bar" << "baz" ).

 I understand that a longer exposure to OOo code raises one pain threshold 
when it comes to bad API, but this really is worse, because it's confusing, 
whereas the two other cases above are rather obvious and the valueOf() case 
is also consistent with OUString usage everywhere else.

> To be honest, I don't think the stated benefits (a different syntax) are
> worth a switch.  The drawbacks I see with this roll-your-own approach,
> compared to building on std::ostream functionality, are:
>
> - Artificially limited to 9 arguments.  You can sure always extend that,
> but it's more work than not having to worry about it.

 Hmm. How often is that going to be needed?

> - Limited to types for which there is a FORMATTER_APPEND_BASED call.  At
> least theoretically, a C++ implementation can e.g., have additional
> numerical types (and use them as, say, std::vector<T>::size_type) for
> which it transparently offers std::ostream inserters.  Again, this can
> always be extended, but it needs to be done.

 And it's trivial work. And it needs to be done for std::ostream for our types 
anyway, since if you had to do it for OUString, the most obvious case, then 
it's just not there yet anyway.

 Moreover, if the format functionality is made specific to the logging case, 
then there can be log formatters even for things that definitely should not 
be causually converted to strings to ostreams but could be used in log 
commands, for example SwNode.

> - No check that there are neither more nor fewer arguments than %Ns (or
> that the set of %Ns spans a range 1--M without holes).

 There's a todo note, not that I think it matters that much if whoever does 
that runs the code at least once.

> > 4) What is the LO policy on char*<->  OUString conversions? It seems to
> > me they always need to be explicit, but I'd prefer to ask.
>
> Yes, always as explicit as suitable, please.  Especially since you
> generally need to specify the text encoding of the char* data, anyway.
> (The current, std::ostream inserter based stuff violates this somewhat,
> offering an inserter for rtl::OUString that implicitly uses
> RTL_TEXTENCODING_UTF8, upon explicit #include
> "rtl/oustringostreaminserter.hxx".)

 Well, that's another advantage of the format approach then. It's an 
inconvenience to have to explicitly say the format string is utf-8 (and 
arguments probably as well), but then this conversion can be again limited 
just to logging and not to every std::ostream operation.

-- 
 Lubos Lunak
 l.lunak@suse.cz