On 01/17/2014 12:50 PM, Sven Wehner wrote:
sorry, I guess I misspoke:
Those examples, which I listed, are rather obvious problems that aren't
translation related.
So let me rephrase:
How to call a developer's attention to small pieces of code?
And especially: may I combine that with translation patches?
I did get that (and think Philipp gave a good response in the
meantime---I'd only add that if you do find something that looks
genuinely broken, best raise it on the dev ML or IRC channel).
I thought "OSL_ENSURE(false, …)" would be "problematic", because firstly there is an
"OSL_FAIL" definition in "include/osl/diagnose.h", which seems to do exactly that.
Secondly, the place I saw this (cui/source/tabpages/numpages.cxx line 2791 ff.)
encapsulates that statement in an if clause.
Together with the error message ("cannot happen"), it implies that could be
reduced from…
--- snip ---
if (SVX_MAX_NUM <= nLvl)
{
OSL_ENSURE(false, "cannot happen.");
return;
}
--- snap ---
… to something like this:
--- snip ---
OSL_ENSURE_RETURN(!(SVX_MAX_NUM <= nLvl), "cannot happen.");
--- snap ---
(with OSL_ENSURE_RETURN being a OSL_ENSURE including a return statement.)
If OSL_ENSURE is deprecated anyway, I guess this is irrelevant anyway.
That specific case should probably be rewritten as either
if (nLvl >= SVX_MAX_NUM) {
SAL_WARN("cui.tabpages", "should never happen");
return;
}
or
assert(nLvl < SVX_MAX_NUM)
depending on context (see
<https://wiki.documentfoundation.org/Development/GeneralProgrammingGuidelines#Assertions_and_Logging>
for details), but there is already EasyHack
<https://bugs.freedesktop.org/show_bug.cgi?id=43157> "Clean up
OSL_ASSERT, DBG_ASSERT, etc.," so no need to call developers' attention
on OSL_ENSURE(false,...) in general.
Stephan
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
