Date: prev next · Thread: first prev next last
2011 Archives by date, by thread · List index


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

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.