[Libreoffice] [PATCH] [PUSHED, partial] Some comments and basic cleanup in sc

[Soeren Moeller]

Hi

Thanks for pushing 0002 and 0004.

In the doxygen documentation on
http://www.stack.nl/~dimitri/doxygen/docblocks.html in the section
"Putting documentation after members" I understand it as if the "<" is
necessary to tell doxygen that the comment relates to the statement
before (i.e. to the left of) the comment, instead of the statement
after (i.e. in the next line), so for instance:

int foo; /**< counting foos */
int bar;

Here doxygen relates "counting foos" as a describtion of the variable
foo, while in

int foo; /** counting foos */
int bar;

doxygen would understand "counting foo" as a describtion of the
variable bar, which would be wrong (same way for ///< resp. ///), did
I miss something?

I used ".\ " not for whitespace size but because it according to the
doxygen manual at http://www.stack.nl/~dimitri/doxygen/docblocks.html
avoids breaking the short description at the ".". Although this only
holds if we use doxygen with JAVADOC_AUTOBRIEF enabled, if not the
brief description should be given in another way. So which short
description type, do we use in Libre Office? (Maybe we should figure
out and announce an offical convention how (and with which parameters)
to use doxygen in LibO and put it on the wiki.)

Best Regards
Sören


2011/1/24 Kohei Yoshida <kyoshida at novell.com>:
> Hi Soeren,
>
> On Sun, 2011-01-23 at 21:01 +0100, Soeren Moeller wrote:
>> Hi
>>
>> Attached four patches:
>> 0001: adding comments to ScFunctionMgr and ScFunctionList in sc/inc/funcdesc.hxx
>
> -  e.g. functions used in formulas in calc
> +  e.g.\ functions used in formulas in calc
>
> I personally find this change a step backward, as this would reduce
> readability in the source code itself.  Let's not this picky about
> whitespace size (assuming that this escaping of the whilespace is used
> for that reason).
>
> -    ::rtl::OUString      *pFuncName;              // Function name
> +    ::rtl::OUString      *pFuncName;              /**< Function name */
> ...
>
> Here, you can simply turn // Function name into /// Function name to
> make it doxygen compliant.  I find /**< .... */ (with the '<') a bit
> ugly, and again this would reduce readability of the comment in the
> source code.  There are code that already uses /// Foo type comments, so
> let's stick with that style.
>
> BTW does doxygen really treat /**< foo */ type comment in any special
> way?  I looked at the doxygen manual, but there is no mention of /**<
> foo */ type comment.  Or did you mean to type /** foo */ type comment
> instead?
>
>> 0002: removing duplicate implementation of
>> ScFunctionMgr::fillLastRecentlyUsedFunctions
>
> Looks good, though I made the following change:
>
> diff --git a/sc/source/ui/formdlg/dwfunctr.cxx b/sc/source/ui/formdlg/dwfunctr.cxx
> index 3175e9a..90e0706 100644
> --- a/sc/source/ui/formdlg/dwfunctr.cxx
> +++ b/sc/source/ui/formdlg/dwfunctr.cxx
> @@ -816,11 +816,11 @@ void ScFunctionDockWin::UpdateFunctionList()
>     }
>     else // LRU-Liste
>     {
> -        for(::std::vector<const formula::IFunctionDescription*>::iterator iter=aLRUList.begin();iter!=aLRUList.end();iter++)
> +        for(::std::vector<const formula::IFunctionDescription*>::iterator iter=aLRUList.begin();iter!=aLRUList.end();++iter)
>         {
> -            const ScFuncDesc* pDesc = static_cast<const ScFuncDesc*>(*iter);
> +            const formula::IFunctionDescription* pDesc = *iter;
>             pAllFuncList->SetEntryData(
> -                    pAllFuncList->InsertEntry( *(pDesc->pFuncName) ),
> +                    pAllFuncList->InsertEntry(pDesc->getFunctionName()),
>                     (void*)pDesc );
>         }
>     }
>
> When using iterator, it's always better to use pre-increment ++iter as
> opposed to post-increment iter++ *unless* the behavior of post-increment
> is called for.  That is not the case in this instance.
>
> Also, you can get the function name string via IFunctionDescription's
> getFunctionName() call, which eliminates the static_cast to ScFuncDesc.
> It's cleaner this way.
>
> This patch is now pushed to master.
>
>> 0003: adding comments to ScFunctionCategory in sc/inc/funcdesc.hxx
>
> This doesn't apply for some reason....  Perhaps depending on 0001?
>
> Also, the same comment as in 0001 applies.  Let's not use /**< foo */
> type comment.
>
>> 0004: cleaning up comments and spacing, and minor translations in
>> sc/source/core/data/funcdesc.cxx
>
> Applied.  Pushed to master.
>
> Could you revise 0001 and 0003 and resend them?  Thanks!
>
> Kohei
>
> --
> Kohei Yoshida, LibreOffice hacker, Calc
> <kyoshida at novell.com>
>
>