Re: Change in core[master]: Replaced deprecated tools/String with OUString in ScMatrix

2012-07-06 Thread Soeren Moeller
I'm using
gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)
on x86_64 GNU/Linux (Ubuntu 12.04 LTS), which runs in a virtual
machine (that hopefully shouldn't make a difference). Hopefully it
doesn't happen which the next patch.

Sören


2012/7/5 Eike Rathke er...@redhat.com:
 Hi Sören,

 On Thursday, 2012-07-05 16:30:39 +, Gerrit wrote:

 From Sören Möller soerenmoeller2...@gmail.com:

 Thanks for fixing the errors, it appears I compiled without --enable-werror 
 and didn't notice the warning on the first one. But I have no idea why I 
 didn't get an error on the second one.

 Indeed, that's odd. What platform/compiler/version do you use? Here
 x86_64 GNU/Linux with gcc 4.6.3

   Eike

 --
 LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
 GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] How to use gprof on LibO?

2011-02-01 Thread Soeren Moeller
Thank you for your answers, I will try both approces.

Oh - fun :-) is a complete expunging of the obsolete types not
 possible ?
I very much hope it is possible, but tools/list is in the codebase
used for both classic list purposes and for situations where the
object is created once, is sorted, and then there is a lot of random
access, so here a vector would be better. But to find out which of
these two cases we are in, profiling could help.

Regards
Sören

2011/1/31 Michael Meeks michael.me...@novell.com:
 Hi Soeren,

 On Sat, 2011-01-29 at 15:50 +0100, Soeren Moeller wrote:
 In our effort of replacing deprecated data types from tools/ by std::
 types we would like to use gprof for profiling (to choose the right
 data types).

        Oh - fun :-) is a complete expunging of the obsolete types not
 possible ?

 But unfortunately, we don't know to set the -pg flag, and where
 the gmon.out file would go. Has anyone of you experiences with
 profiling LibO, and would like to tell us, how to do?

        I always use callgrind instead; and I use it like this:

 export OOO_DISABLE_RECOVERY=1
 valgrind --tool=callgrind --simulate-cache=yes --dump-instr=yes ./soffice.bin 
 -writer -splash-pipe=0

        Then I use 'kcachegrind' on the output.

        Now - possibly you want to use callgrind_control -z just before you
 start doing something fun in calc - so that you can not get swamped by
 the startup cost; and of course - this method is fairly slow, and really
 you need to build with debug symbols to get a good view of what is up.

        OTOH - it is a wonderful tool ;-)

        HTH,

                Michael.

 --
  michael.me...@novell.com  , Pseudo Engineer, itinerant idiot



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


[Libreoffice] How to use gprof on LibO?

2011-01-29 Thread Soeren Moeller
Hi

In our effort of replacing deprecated data types from tools/ by std::
types we would like to use gprof for profiling (to choose the right
data types). But unfortunately, we don't know to set the -pg flag, and
where the gmon.out file would go. Has anyone of you experiences with
profiling LibO, and would like to tell us, how to do?

Best regards
Sören
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] Replaced tools/list with std::list in ScAddInListener

2011-01-29 Thread Soeren Moeller
Hi

Now we have replaced tools/list with std::list in ScAddInListener,
here we used a list, as there is adding, removing, and searching on
this list, and no clear way to sort the elements. The patch comes in
three files:
0001 Replacing deprecated data types from solar.h
0002 Cleaning up the spacings
0003 Replacing tools/list with std::list
and a non-related 4th file:
0004 Removing unnecessary include of tools/string in funcdesc.hxx

The patches compile fine, but we couldn't really test them in the
running scalc, as it was impossible for us, to get scalc to use the
ScAddInListener (although it seems to be used in the unit tests
without failing).

Best Regards
Sören Möller
From f1fe93b0b5a2edd1e6da8b1629c5d16651e6bb02 Mon Sep 17 00:00:00 2001
From: Thies Pierdola thiespierd...@gmail.com
Date: Sat, 29 Jan 2011 17:39:34 +0100
Subject: [PATCH] Replaced deprecated types with sal types

---
 sc/source/core/inc/addinlis.hxx  |2 +-
 sc/source/core/tool/addinlis.cxx |   12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/sc/source/core/inc/addinlis.hxx b/sc/source/core/inc/addinlis.hxx
index d3cb130..9c8b471 100644
--- a/sc/source/core/inc/addinlis.hxx
+++ b/sc/source/core/inc/addinlis.hxx
@@ -72,7 +72,7 @@ public:
 com::sun::star::sheet::XVolatileResult xVR );
 static voidRemoveDocument( ScDocument* pDocument );
 
-BOOL	HasDocument( ScDocument* pDoc ) const	{ return pDocs-Seek_Entry( pDoc ); }
+bool	HasDocument( ScDocument* pDoc ) const	{ return pDocs-Seek_Entry( pDoc ); }
 void	AddDocument( ScDocument* pDoc )			{ pDocs-Insert( pDoc ); }
 const com::sun::star::uno::Any GetResult() const{ return aResult; }
 
diff --git a/sc/source/core/tool/addinlis.cxx b/sc/source/core/tool/addinlis.cxx
index 71b3d72..81fdd80 100644
--- a/sc/source/core/tool/addinlis.cxx
+++ b/sc/source/core/tool/addinlis.cxx
@@ -84,8 +84,8 @@ ScAddInListener* ScAddInListener::Get( uno::Referencesheet::XVolatileResult xV
 {
 sheet::XVolatileResult* pComp = xVR.get();
 
-ULONG nCount = aAllListeners.Count();
-for (ULONG nPos=0; nPosnCount; nPos++)
+sal_uInt32 nCount = aAllListeners.Count();
+for (sal_uInt32 nPos=0; nPosnCount; nPos++)
 {
 ScAddInListener* pLst = (ScAddInListener*)aAllListeners.GetObject(nPos);
 if ( pComp == (sheet::XVolatileResult*)pLst-xVolRes.get() )
@@ -97,14 +97,14 @@ ScAddInListener* ScAddInListener::Get( uno::Referencesheet::XVolatileResult xV
 //!	move to some container object?
 void ScAddInListener::RemoveDocument( ScDocument* pDocumentP )
 {
-ULONG nPos = aAllListeners.Count();
+sal_uInt32 nPos = aAllListeners.Count();
 while (nPos)
 {
 //	loop backwards because elements are removed
 --nPos;
 ScAddInListener* pLst = (ScAddInListener*)aAllListeners.GetObject(nPos);
 ScAddInDocs* p = pLst-pDocs;
-USHORT nFoundPos;
+sal_uInt16 nFoundPos;
 if ( p-Seek_Entry( pDocumentP, nFoundPos ) )
 {
 p-Remove( nFoundPos );
@@ -140,8 +140,8 @@ void SAL_CALL ScAddInListener::modified( const ::com::sun::star::sheet::ResultEv
 Broadcast( ScHint( SC_HINT_DATACHANGED, ScAddress(), NULL ) );
 
 const ScDocument** ppDoc = (const ScDocument**) pDocs-GetData();
-USHORT nCount = pDocs-Count();
-for ( USHORT j=0; jnCount; j++, ppDoc++ )
+sal_uInt16 nCount = pDocs-Count();
+for ( sal_uInt16 j=0; jnCount; j++, ppDoc++ )
 {
 ScDocument* pDoc = (ScDocument*)*ppDoc;
 pDoc-TrackFormulas();
-- 
1.7.0.4

From 8876c079f754211c2ba5ab8fa959c1d3056eceff Mon Sep 17 00:00:00 2001
From: Thies Pierdola thiespierd...@gmail.com
Date: Sat, 29 Jan 2011 16:17:15 +0100
Subject: [PATCH 2/3] Cleaned up spacings

---
 sc/source/core/inc/addinlis.hxx  |   70 +++---
 sc/source/core/tool/addinlis.cxx |   42 +++
 2 files changed, 48 insertions(+), 64 deletions(-)

diff --git a/sc/source/core/inc/addinlis.hxx b/sc/source/core/inc/addinlis.hxx
index 9c8b471..ace8d49 100644
--- a/sc/source/core/inc/addinlis.hxx
+++ b/sc/source/core/inc/addinlis.hxx
@@ -29,18 +29,15 @@
 #ifndef SC_ADDINLIS_HXX
 #define SC_ADDINLIS_HXX
 
-#include adiasync.hxx			// for ScAddInDocs PtrArr
+#include adiasync.hxx // for ScAddInDocs PtrArr
 #include tools/list.hxx
 #include com/sun/star/sheet/XResultListener.hpp
 #include com/sun/star/sheet/XVolatileResult.hpp
 #include com/sun/star/lang/XServiceInfo.hpp
 #include cppuhelper/implbase2.hxx
 
-
-
 class ScDocument;
 
-
 class ScAddInListener : public cppu::WeakImplHelper2
 com::sun::star::sheet::XResultListener,
 com::sun::star::lang::XServiceInfo ,
@@ -48,53 +45,56 @@ class ScAddInListener : public cppu::WeakImplHelper2
 {
 private:
 com::sun::star::uno::Referencecom::sun::star::sheet::XVolatileResult xVolRes;
-com::sun::star::uno::Any	aResult;
-ScAddInDocs*		

Re: [Libreoffice] Patches by two persons?

2011-01-28 Thread Soeren Moeller
Thank you for your answers, for now we will alternate the author in
the git patch files, but I will mostly send the mails to this mailing
list. (And we now both have send a mail, confirming that all our
patches are LGPLv3+/MPL)

Best regards
Sören

2011/1/28 Norbert Thiebaud nthieb...@gmail.com:
 On Fri, Jan 28, 2011 at 3:15 AM, Michael Meeks michael.me...@novell.com 
 wrote:

 On Fri, 2011-01-28 at 10:02 +0100, Cedric Bosdonnat wrote:
 Well, we may need to adapt the tools computing the stats and credits to
 take this into account. We will need to get all the names pretty easily
 from git to get everyone properly thanked. Isn't the sign-off way the
 best one?

        Signed off typically means, not I also wrote this, but I reviewed
 this, and I certify its origin - I think we want to retain the latter
 meaning.

        So - I would be inclined to have some different way just for Soeren 
 and
 his (thus far anonymous) friend ;-) either that - or we encourage them
 to alternate credits in their patches (which might be easier) :-)

 Yeah, I think alternate credit is probably the easiest route here.

 Norbert


        HTH,

                Michael.

 --
  michael.me...@novell.com  , Pseudo Engineer, itinerant idiot


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

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

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


Re: [Libreoffice] [PATCH] Replaced tools/list with std::vector in funcdesc.hxx / funcdesc.cxx

2011-01-28 Thread Soeren Moeller
Yet another patch, removing use of tools/list from ScFunctionList,
too. In this class there should be a clear performance gain, by using
a vector instead of a list, as our tests revealed that the random
access Get() function is called 2-3000 times for each use of a
spreadsheet function. Note that we use a std::list for constructing
the object, as the number of elements is unknown while constructing,
but then convert the list to a vector at the end of the construction,
to make efficient random access possible.

With this patch (together with our last patch)
funcdesc.hxx/funcdesc.cxx shouldn't use tools/list anymore, so we have
removed the includes.
The patch compiles fine here, and scalc seems to work as expected.

Regards
Sören Möller

2011/1/28 Soeren Moeller soerenmoeller2...@gmail.com:
 Hi

 In the attached patches (0001 is the real patch, 0002 contains three
 comments missing in 0001) we have replaced use of tools/list by use of
 std::vector in ScFunctionMgr and ScFunctionCategory (in
 sc/inc/funcdesc.hxx and sc/source/core/funcdesc.cxx). We choose
 vector, as there is done random access to the lists, while they are
 only modified once (at creation). We changed the behaviour of
 ScFunctionMgr slightly, as Get() now no longer resets the iterators
 for First() and Next(), as this is quite counter intuitive. The patch
 builds fine, and the behaviour of scalc seems unchanged.

 The patch is joint work with Thies Pierdola, who is marked as author
 in the patch file.

 Regards
 Sören Möller
 (LGPLv3+/MPL for this and future patches)

From c9b432c08606761130a033e141d0e068d92b681d Mon Sep 17 00:00:00 2001
From: Thies Pierdola thiespierd...@gmail.com
Date: Fri, 28 Jan 2011 21:56:06 +0100
Subject: [PATCH] Replaced tools/list by std::vector in ScFunctionList

As the random access Get is called a couple of thousand times each time
a spreadsheet function is used, the use of vector should have a huge
performance gain.
---
 sc/inc/funcdesc.hxx  |   29 ---
 sc/source/core/data/funcdesc.cxx |   48 +
 2 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index 49909c5..8a2404d 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -29,14 +29,10 @@
 #ifndef SC_FUNCDESC_HXX
 #define SC_FUNCDESC_HXX
 
-/* Function descriptions for function wizard / autopilot / most recent used
- * list et al. Separated from the global.hxx lump, implementation still in
- * global.cxx
- */
+/* Function descriptions for function wizard / autopilot */
 
 #include scfuncs.hrc
 
-#include tools/list.hxx
 #include formula/IFunctionDescription.hxx
 #include sal/types.h
 #include rtl/ustring.hxx
@@ -240,26 +236,21 @@ public:
 ScFunctionList();
 ~ScFunctionList();
 
-sal_uInt32   GetCount() const
-{ return aFunctionList.Count(); }
+sal_uInt32 GetCount() const
+   { return aFunctionList.size(); }
 
-const ScFuncDesc*   First()
-{ return (const ScFuncDesc*) aFunctionList.First(); }
+const ScFuncDesc* First();
 
-const ScFuncDesc*   Next()
-{ return (const ScFuncDesc*) aFunctionList.Next(); }
+const ScFuncDesc* Next();
 
-const ScFuncDesc* GetFunction( sal_uInt32 nIndex ) const
-{ return static_castconst ScFuncDesc*(aFunctionList.GetObject(nIndex)); }
+const ScFuncDesc* GetFunction( sal_uInt32 nIndex ) const;
 
-ScFuncDesc* GetFunction( sal_uInt32 nIndex )
-{ return static_castScFuncDesc*(aFunctionList.GetObject(nIndex)); }
-
-xub_StrLen  GetMaxFuncNameLen() const
-{ return nMaxFuncNameLen; }
+xub_StrLen GetMaxFuncNameLen() const
+   { return nMaxFuncNameLen; }
 
 private:
-ListaFunctionList; /** List of functions */
+::std::vectorconst ScFuncDesc* aFunctionList; /** List of functions */
+::std::vectorconst ScFuncDesc*::iterator aFunctionListIter; /** position in function list */
 xub_StrLen  nMaxFuncNameLen; /** Length of longest function name */
 };
 
diff --git a/sc/source/core/data/funcdesc.cxx b/sc/source/core/data/funcdesc.cxx
index 892d643..b3a502c 100644
--- a/sc/source/core/data/funcdesc.cxx
+++ b/sc/source/core/data/funcdesc.cxx
@@ -39,7 +39,6 @@
 
 #include rtl/ustring.hxx
 #include rtl/ustrbuf.hxx
-#include tools/list.hxx
 #include tools/rcid.h
 #include tools/resid.hxx
 #include tools/string.hxx
@@ -378,14 +377,13 @@ ScFunctionList::ScFunctionList() :
 ScFuncDesc* pDesc = NULL;
 xub_StrLen nStrLen = 0;
 FuncCollection* pFuncColl;
+::std::listScFuncDesc* tmpFuncList;
 sal_uInt16 nDescBlock[] =
 {
 RID_SC_FUNCTION_DESCRIPTIONS1,
 RID_SC_FUNCTION_DESCRIPTIONS2
 };
 
-aFunctionList.Clear();
-
 for (sal_uInt16 k = 0; k  SAL_N_ELEMENTS(nDescBlock); ++k)
 {
 ::std::auto_ptrScResourcePublisher pBlock( new ScResourcePublisher( ScResId( nDescBlock

Re: [Libreoffice] Crash in master: Sum button in sc

2011-01-26 Thread Soeren Moeller
Thanks for the answer. I now used gdb to locate the segfault, and
found it to happen when ScViewFunc called ScRangeList.front() with an
empty range list, which resulted in a segfault while trying to
retrieve the first element of the list. I have prepared the attached
patch, adding a check for empty list in ScViewFunc. (As it here
opposed to ScRangeList, was possible to check that not returning an
element was handled ok.) This removes the crash, and the sum button
seems to work fine again. Although it maybe still would make sense to
check the other uses of ScRangeList.front() for similar problems, I
will try to look into this later on, but this patch shoul at least
make this use safe.

Regards
Sören Möller
(LGPLv3+ / MPL)


2011/1/26 Kohei Yoshida kyosh...@novell.com:
 Hi Soeren,

 On Tue, 2011-01-25 at 23:13 +0100, Soeren Moeller wrote:
 Hi

 I just noticed a bug in master (pull about 1h ago):
 When I in calc press the sum button (shaped like a sigma), then calc
 crashes instantly and without any message. This seems to happen only
 when calc doesn't guess a sum area by itself. (E.g. if there are
 numbers in the cells above the marked cell, calc suggest those cells
 as a sum area, and it works fine, but if there are no cells with
 numbers nearby it crashes). Using the sum function through the
 Function Wizard works fine, though.

 Well, I'm not surprised; after all it's master where things are fairly
 unstable, and crashes are common occurrences. :-)

 Having said that, it's good to catch these nasty crashes early, and I'm
 glad you brought this issue to light.

 Are you familiar with getting stack traces with gdb?  That would be the
 first step when investigating a crash.  To get a meaningful stack trace
 you need to re-build the sc module with debug symbol compiled in.

 http://wiki.documentfoundation.org/Development/Native_Build#Partial_debug_build

 Once done, start Calc, attach gdb, reproduce the crash and get a
 backtrace in gdb.  That will give you the location of the crash from
 which you can sniff around the code to see what's going wrong.

 I'm just getting used to the codebase, so it would be nice if someone
 who is more into it would take a look, and check if the error can be
 reproduced by others. I can reproduce it even after a make clean; ./g
 pull; make; make dev-install.

 Yup, this is perfectly reproducible.

 Anyway, it would be great if you could get a backtrace output from gdb
 to see where the crash occurs.

 Let me know if you need additional help.

 Best,

 Kohei

 --
 Kohei Yoshida, LibreOffice hacker, Calc
 kyosh...@novell.com


From cc930a1a9ee473bc6b827f5456bf33126b728cc3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6ren=20M=C3=B6ller?= soerenmoeller2...@gmail.com
Date: Wed, 26 Jan 2011 23:48:33 +0100
Subject: [PATCH] Added check for empty rRangeList in ScViewFunc

This fixes a crash, which occured when the sum function was called, without
beeing able to generate an automatic range.
---
 sc/source/ui/view/viewfun2.cxx |   24 +---
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/sc/source/ui/view/viewfun2.cxx b/sc/source/ui/view/viewfun2.cxx
index 99a5c9c..34e7694 100644
--- a/sc/source/ui/view/viewfun2.cxx
+++ b/sc/source/ui/view/viewfun2.cxx
@@ -744,17 +744,19 @@ String ScViewFunc::GetAutoSumFormula( const ScRangeList rRangeList, bool bSubTo
 pArray-AddOpCode(ocSep);
 }
 
-ScRangeList aRangeList = rRangeList;
-const ScRange* pFirst = aRangeList.front();
-size_t ListSize = aRangeList.size();
-for ( size_t i = 0; i  ListSize; ++i )
-{
-const ScRange* p = aRangeList[i];
-if (p != pFirst)
-pArray-AddOpCode(ocSep);
-ScComplexRefData aRef;
-aRef.InitRangeRel(*p, rAddr);
-pArray-AddDoubleReference(aRef);
+if(!rRangeList.empty()){
+ScRangeList aRangeList = rRangeList;
+const ScRange* pFirst = aRangeList.front();
+size_t ListSize = aRangeList.size();
+for ( size_t i = 0; i  ListSize; ++i )
+{
+const ScRange* p = aRangeList[i];
+if (p != pFirst)
+pArray-AddOpCode(ocSep);
+ScComplexRefData aRef;
+aRef.InitRangeRel(*p, rAddr);
+pArray-AddDoubleReference(aRef);
+}
 }
 
 pArray-AddOpCode(ocClose);
-- 
1.7.0.4

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


[Libreoffice] Crash in master: Sum button in sc

2011-01-25 Thread Soeren Moeller
Hi

I just noticed a bug in master (pull about 1h ago):
When I in calc press the sum button (shaped like a sigma), then calc
crashes instantly and without any message. This seems to happen only
when calc doesn't guess a sum area by itself. (E.g. if there are
numbers in the cells above the marked cell, calc suggest those cells
as a sum area, and it works fine, but if there are no cells with
numbers nearby it crashes). Using the sum function through the
Function Wizard works fine, though.

I'm just getting used to the codebase, so it would be nice if someone
who is more into it would take a look, and check if the error can be
reproduced by others. I can reproduce it even after a make clean; ./g
pull; make; make dev-install.

I'm using LibO under Ubuntu 10.04, compiling with standard parameters
(apart from num-cpu and max-jobs).

Best regards
Sören Möller
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2011-01-24 Thread 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 kyosh...@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::vectorconst formula::IFunctionDescription*::iterator 
 iter=aLRUList.begin();iter!=aLRUList.end();iter++)
 +        for(::std::vectorconst formula::IFunctionDescription*::iterator 
 iter=aLRUList.begin();iter!=aLRUList.end();++iter)
         {
 -            const ScFuncDesc* pDesc = static_castconst 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
 kyosh...@novell.com


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


[Libreoffice] [PATCH] Some comments and basic cleanup in sc

2011-01-23 Thread Soeren Moeller
Hi

Attached four patches:
0001: adding comments to ScFunctionMgr and ScFunctionList in sc/inc/funcdesc.hxx
0002: removing duplicate implementation of
ScFunctionMgr::fillLastRecentlyUsedFunctions
0003: adding comments to ScFunctionCategory in sc/inc/funcdesc.hxx
0004: cleaning up comments and spacing, and minor translations in
sc/source/core/data/funcdesc.cxx

please apply

Best regards
Sören Möller
(LGPLv3+ / MPL)
From d6b6a1abbb2afa70a0fb5d45bb7dd19f57c60f91 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6ren=20M=C3=B6ller?= soerenmoeller2...@gmail.com
Date: Sat, 22 Jan 2011 21:06:03 +0100
Subject: [PATCH 1/4] Added doxygen style comments to ScFunctionMgr and ScFunctionList

---
 sc/inc/funcdesc.hxx |  173 ---
 1 files changed, 136 insertions(+), 37 deletions(-)

diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index 05c71e9..3b51fce 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -50,7 +50,7 @@ class ScFunctionMgr;
 
 /**
   Stores and generates human readable descriptions for spreadsheet-functions,
-  e.g. functions used in formulas in calc
+  e.g.\ functions used in formulas in calc
 */
 class ScFuncDesc : public formula::IFunctionDescription
 {
@@ -190,31 +190,37 @@ public:
 */
 struct ParameterFlags
 {
-boolbOptional   :1; // Parameter is optional
-boolbSuppress   :1; // Suppress parameter in UI because not implemented yet
+boolbOptional   :1; /** Parameter is optional */
+boolbSuppress   :1; /** Suppress parameter in UI because not implemented yet */
 
 ParameterFlags() : bOptional(false), bSuppress(false) {}
 };
 
 
 
-::rtl::OUString  *pFuncName;  // Function name
-::rtl::OUString  *pFuncDesc;  // Description of function
-::rtl::OUString **ppDefArgNames;  // Parameter name(s)
-::rtl::OUString **ppDefArgDescs;  // Description(s) of parameter(s)
-ParameterFlags   *pDefArgFlags;   // Flags for each parameter
-sal_uInt16nFIndex;// Unique function index
-sal_uInt16nCategory;  // Function category
-sal_uInt16nArgCount;  // All parameter count, suppressed and unsuppressed
-sal_uInt16nHelpId;// HelpId of function
-bool  bIncomplete :1; // Incomplete argument info (set for add-in info from configuration)
-bool  bHasSuppressedArgs  :1; // Whether there is any suppressed parameter.
+::rtl::OUString  *pFuncName;  /** Function name */
+::rtl::OUString  *pFuncDesc;  /** Description of function */
+::rtl::OUString **ppDefArgNames;  /** Parameter name(s) */
+::rtl::OUString **ppDefArgDescs;  /** Description(s) of parameter(s) */
+ParameterFlags   *pDefArgFlags;   /** Flags for each parameter */
+sal_uInt16nFIndex;/** Unique function index */
+sal_uInt16nCategory;  /** Function category */
+sal_uInt16nArgCount;  /** All parameter count, suppressed and unsuppressed */
+sal_uInt16nHelpId;/** HelpId of function */
+bool  bIncomplete :1; /** Incomplete argument info (set for add-in info from configuration) */
+bool  bHasSuppressedArgs  :1; /** Whether there is any suppressed parameter. */
 };
 
-
-
-//
-
+/**
+  List of spreadsheet functions.
+  Generated by retrieving functions from resources, AddIns and StarOne AddIns,
+  and storing these in one linked list. Functions can be retrieved by index and
+  by iterating through the list, starting at the First element, and retrieving
+  the Next elements one by one.
+
+  The length of the longest function name can be retrieved for easier
+  processing (i.e printing a function list).
+*/
 class ScFunctionList
 {
 public:
@@ -240,11 +246,13 @@ public:
 { return nMaxFuncNameLen; }
 
 private:
-ListaFunctionList;
-xub_StrLen  nMaxFuncNameLen;
+ListaFunctionList; /** List of functions */
+xub_StrLen  nMaxFuncNameLen; /** Length of longest function name */
 };
 
-//
+/**
+  Category of spreadsheet functions.
+*/
 class ScFunctionCategory : public formula::IFunctionCategory
 {
 ScFunctionMgr* m_pMgr;
@@ -260,34 +268,125 @@ public:
 virtual sal_uInt32  getNumber() const;
 virtual ::rtl::OUString getName() const;
 };
-//
+
 #define SC_FUNCGROUP_COUNT  ID_FUNCTION_GRP_ADDINS
+/**
+  Stores 

Re: [Libreoffice] Splitting up source/header files for clarity

2011-01-11 Thread Soeren Moeller
I have now splitted the implementation of funcdesc.hxx out into a new
source file funcdesc.cxx. In the process I also have extracted two new
header files for classes previously defined in global.cxx. I have
corrected all includes and the result compiles for me, I have added
the new source file to the relevant makefile.mk. Further I have added
comments to the first class in funcdesc.hxx, I hope to document the
others in a later patch. Please review my patch and apply if ok.

Best Regards
Sören Möller
(LGPLv3+ / MPL)

2011/1/10 Tor Lillqvist tlillqv...@novell.com:
 - Is it best to make one file for each class, or keep them together
 and make one funcdesc.cxx (containing four classes)?

 I'd say one file, funcdesc.cxx, corresponding to funcdesc.hxx. But that is 
 just my personal opinion.

 - Should the license header for the new file(s) be a copy of the
 license header from global.cxx (as the code is copied out), or the new
 Libo header?

 As the code is the same (just rearranged into different files), the license 
 header should be the same.

 - Is it enough to add the new file(s) to
 sc/source/core/data/makefile.mk for building,

 Yes, that should be enough for your testing; if/when you then want to make a 
 patch out of it, you need to git add the new files to your local repository 
 clone.)

 --tml



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


[Libreoffice] [PATCH] Changed String to OUString in ScFuncDesc

2011-01-09 Thread Soeren Moeller
Hi

Please review and apply if appropriate this patch:
I have replaced four public (deprecated) String variables of
ScFuncDesc in sc/inc/funcdesc.hxx by OUString and changed all uses of
these variables to use OUString instead. Most places by using relevant
methods for OUString, and a few places (where a String is retrieved
using a ResId), by casting the String to an OUString.

funcdesc.hxx and its implementation in global.cxx still contain other
uses of deprecated datatypes, but I hope to change these in a later
patch.

Regards
Sören Möller
(LGPLv3+ / MPL)
From d8c5ec79dff93bbc3c101e64bd59907527ee193f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6ren=20M=C3=B6ller?= soerenmoeller2...@gmail.com
Date: Sun, 9 Jan 2011 18:12:47 +0100
Subject: [PATCH] Changed String to OUString in public variables of ScFuncDesc

I have changed four public variables of ScFuncDesc in sc/inc/funcdesc.hxx from deprecated String to OUString and corrected all uses of these variables
---
 sc/inc/appluno.hxx|1 +
 sc/inc/funcdesc.hxx   |   22 ++--
 sc/source/core/data/global.cxx|   72 ++--
 sc/source/core/tool/addincol.cxx  |   16 
 sc/source/ui/formdlg/dwfunctr.cxx |2 +-
 sc/source/ui/unoobj/appluno.cxx   |   21 +--
 6 files changed, 67 insertions(+), 67 deletions(-)

diff --git a/sc/inc/appluno.hxx b/sc/inc/appluno.hxx
index 681dd8d..f8d82cd 100644
--- a/sc/inc/appluno.hxx
+++ b/sc/inc/appluno.hxx
@@ -38,6 +38,7 @@
 #include com/sun/star/container/XNameAccess.hpp
 #include cppuhelper/implbase2.hxx
 #include cppuhelper/implbase4.hxx
+#include rtl/ustring.hxx
 
 class ScFunctionDescriptionObj;
 
diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index 3640bfc..c07dda0 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -93,17 +93,17 @@ public:
 suppressed). */
 USHORT  GetSuppressedArgCount() const;
 
-String  *pFuncName;  // Function name
-String  *pFuncDesc;  // Description of function
-String **ppDefArgNames;  // Parameter name(s)
-String **ppDefArgDescs;  // Description(s) of parameter(s)
-ParameterFlags  *pDefArgFlags;   // Flags for each parameter
-USHORT   nFIndex;// Unique function index
-USHORT   nCategory;  // Function category
-USHORT   nArgCount;  // All parameter count, suppressed and unsuppressed
-USHORT   nHelpId;// HelpID of function
-bool bIncomplete :1; // Incomplete argument info (set for add-in info from configuration)
-bool bHasSuppressedArgs  :1; // Whether there is any suppressed parameter.
+::rtl::OUString  *pFuncName;  // Function name
+::rtl::OUString  *pFuncDesc;  // Description of function
+::rtl::OUString **ppDefArgNames;  // Parameter name(s)
+::rtl::OUString **ppDefArgDescs;  // Description(s) of parameter(s)
+ParameterFlags   *pDefArgFlags;   // Flags for each parameter
+USHORTnFIndex;// Unique function index
+USHORTnCategory;  // Function category
+USHORTnArgCount;  // All parameter count, suppressed and unsuppressed
+USHORTnHelpId;// HelpID of function
+bool  bIncomplete :1; // Incomplete argument info (set for add-in info from configuration)
+bool  bHasSuppressedArgs  :1; // Whether there is any suppressed parameter.
 };
 
 //
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index 93550a9..e46a3e6 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -1168,17 +1168,17 @@ ScFuncRes::ScFuncRes( ResId aRes, ScFuncDesc* pDesc, bool  rbSuppressed )
 }
 }
 
-pDesc-pFuncName = new String( ScCompiler::GetNativeSymbol( static_castOpCode( aRes.GetId(;
-pDesc-pFuncDesc = new String(ScResId(1));
+pDesc-pFuncName = new ::rtl::OUString( ScCompiler::GetNativeSymbol( static_castOpCode( aRes.GetId(;
+pDesc-pFuncDesc = (::rtl::OUString*)(new String(ScResId(1)));
 
 if (nArgs)
 {
-pDesc-ppDefArgNames = new String*[nArgs];
-pDesc-ppDefArgDescs = new String*[nArgs];
+pDesc-ppDefArgNames = new ::rtl::OUString*[nArgs];
+pDesc-ppDefArgDescs = new ::rtl::OUString*[nArgs];
 for (USHORT i = 0; i  nArgs; i++)
 {
-pDesc-ppDefArgNames[i] = new String(ScResId(2*(i+1)  ));
-pDesc-ppDefArgDescs[i] = new String(ScResId(2*(i+1)+1));
+pDesc-ppDefArgNames[i] = (::rtl::OUString*)(new String(ScResId(2*(i+1)  )));
+pDesc-ppDefArgDescs[i] = 

[Libreoffice] [PATCH] Changed String to OUString in funcdesc.hxx

2011-01-09 Thread Soeren Moeller
And here another patch for sc/inc/funcdesc.hxx and related source
files, changing even more occurrences of (deprecated) String to
OUString. I also changed the uses of these variables/functions to use
OUString instead (using casts when forced to use String originally
because of ResId).

Please review and apply
Regards
Sören Möller
(LGPLv3+ / MPL)
From a2ff17423179900f4d0fa28243a91bca67cdbc52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6ren=20M=C3=B6ller?= soerenmoeller2...@gmail.com
Date: Sun, 9 Jan 2011 20:30:43 +0100
Subject: [PATCH] Changed String to OUString in funcdesc.hxx

I have changed variables and functions in sc/inc/funcdesc.hxx to use OUString instead of deprecated String, and I have also changed all uses of these variables/functions to use OUString
---
 sc/inc/funcdesc.hxx   |7 ++--
 sc/source/core/data/global.cxx|   64 +---
 sc/source/ui/app/inputhdl.cxx |2 +-
 sc/source/ui/formdlg/dwfunctr.cxx |2 +-
 4 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/sc/inc/funcdesc.hxx b/sc/inc/funcdesc.hxx
index c07dda0..edc50df 100644
--- a/sc/inc/funcdesc.hxx
+++ b/sc/inc/funcdesc.hxx
@@ -57,6 +57,7 @@ public:
 parameters only one element is added to the end of the sequence. */
 virtual void fillVisibleArgumentMapping(::std::vectorUSHORT _rArguments) const ;
 virtual void initArgumentInfo()  const;
+/** Returns the full function signature: FUNCTIONNAME( parameter list ). */
 virtual ::rtl::OUString getSignature() const ;
 virtual long getHelpId() const ;
 
@@ -81,9 +82,7 @@ public:
 voidClear();
 
 /** Returns a semicolon separated list of all parameter names. */
-String  GetParamList() const;
-/** Returns the full function signature: FUNCTIONNAME( parameter list ). */
-String  GetSignature() const;
+::rtl::OUString  GetParamList() const;
 
 
 
@@ -158,7 +157,7 @@ public:
 ScFunctionMgr();
 virtual ~ScFunctionMgr();
 
-static String   GetCategoryName(sal_uInt32 _nCategoryNumber );
+static ::rtl::OUString   GetCategoryName(sal_uInt32 _nCategoryNumber );
 
 const ScFuncDesc*   Get( const String rFName ) const;
 const ScFuncDesc*   Get( USHORT nFIndex ) const;
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index e46a3e6..b3de8ba 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -1457,11 +1457,11 @@ void ScFuncDesc::Clear()
 
 //
 
-String ScFuncDesc::GetParamList() const
+::rtl::OUString ScFuncDesc::GetParamList() const
 {
 const String sep = ScCompiler::GetNativeSymbol(ocSep);
 
-String aSig;
+::rtl::OUString aSig;
 
 if ( nArgCount  0 )
 {
@@ -1476,19 +1476,19 @@ String ScFuncDesc::GetParamList() const
 else
 {
 nLastAdded = i;
-aSig += (String)*(ppDefArgNames[i]);
+aSig += *(ppDefArgNames[i]);
 if ( i != nArgCount-1 )
 {
-aSig.Append(sep);
-aSig.AppendAscii(RTL_CONSTASCII_STRINGPARAM(   ));
+aSig += ::rtl::OUString(sep);
+aSig += ::rtl::OUString::createFromAscii(   );
 }
 }
 }
 // If only suppressed parameters follow the last added parameter,
 // remove one ; 
 if (nLastSuppressed  nArgCount  nLastAdded  nLastSuppressed 
-aSig.Len() = 2)
-aSig.Erase( aSig.Len() - 2 );
+aSig.getLength() = 2)
+aSig = aSig.copy(0,aSig.getLength() - 2);
 }
 else
 {
@@ -1497,23 +1497,23 @@ String ScFuncDesc::GetParamList() const
 {
 if (!pDefArgFlags[nArg].bSuppress)
 {
-aSig += (String)*(ppDefArgNames[nArg]);
-aSig.Append(sep);
-aSig.AppendAscii(RTL_CONSTASCII_STRINGPARAM(   ));
+aSig += *(ppDefArgNames[nArg]);
+aSig += ::rtl::OUString(sep);
+aSig += ::rtl::OUString::createFromAscii(   );
 }
 }
 /* NOTE: Currently there are no suppressed var args parameters. If
  * there were, we'd have to cope with it here and above for the fix
  * parameters. For now parameters are always added, so no special
  * treatment of a trailing ;  necessary. */
-aSig += (String)*(ppDefArgNames[nFix]);
-aSig += '1';
-aSig.Append(sep);
-aSig.AppendAscii(RTL_CONSTASCII_STRINGPARAM(   ));
-aSig += (String)*(ppDefArgNames[nFix]);
-aSig += '2';
-aSig.Append(sep);
-  

[Libreoffice] Splitting up source/header files for clarity

2011-01-09 Thread Soeren Moeller
Hi

I have been working with sc/inc/funcdesc.hxx, and this header defines
four closely related classes, but all the implementation of these
classes is in sc/source/core/data/global.cxx together with the
implementation of a number of other classes. This makes it confusing
to find the implementations and check these classes separately, so it
would be nice to have these implementations in their own file/files.
This is even suggested by a comment in sc/inc/funcdesc.hxx which
sounds, as if the header earlier was part of some huge file too.
Moving these implementations to own files raises some questions:
- Is it best to make one file for each class, or keep them together
and make one funcdesc.cxx (containing four classes)?
- Should the license header for the new file(s) be a copy of the
license header from global.cxx (as the code is copied out), or the new
Libo header?
- Is it enough to add the new file(s) to
sc/source/core/data/makefile.mk for building, or have they to be added
somewhere else too?

Any comments are appreciated

Best Regards
Sören Möller
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] Removed dependencies on tools/solar.h in drawpage.hxx/.cxx

2011-01-05 Thread Soeren Moeller
Hi and thank you for your comments

2011/1/5 Kohei Yoshida kyosh...@novell.com:
 Hi Sören,

 Thanks for the patch.  Is there a reason why this one is not an
 attachment? :-)
It is not an attachment as I used git send-email to send it to the
mailing list, but the result seems a bit wierd when reading the list,
and it is not possible to give extra explanations in the mail when
sending the patch, so I think I will go back to attachments for my
next commits.


 Anyway, I have some comments.

 On Tue, 2011-01-04 at 20:20 +0100, Sören Möller wrote:
 Replaced datatypes from tools/solar.h with corresponding types from 
 sal/types.h in drawpage.hxx/.cxx and added missing include of sal/types.h in 
 docparam.hxx
 ---
  sc/inc/docparam.hxx              |    1 +
  sc/inc/drawpage.hxx              |    4 ++--
  sc/source/core/data/drawpage.cxx |    2 +-
  3 files changed, 4 insertions(+), 3 deletions(-)

 diff --git a/sc/inc/docparam.hxx b/sc/inc/docparam.hxx
 index 2511f26..01170a5 100644
 --- a/sc/inc/docparam.hxx
 +++ b/sc/inc/docparam.hxx
 @@ -32,6 +32,7 @@
  #ifndef SC_DOCPARAM_HXX
  #define SC_DOCPARAM_HXX

 +#include sal/types.h
  #include address.hxx

 This should not be necessary (though it won't hurt) as the address.hxx
 already includes sal types.  Does it not build without this change for
 you?
It does build without the change for me, I did the change to ensure
that we don't break the file accidentially by changing includes of
address.hxx, although I now can see that this maybe isn't necessary
for such basic includes (types are quite standard and almost everyone
includes address.hxx). I think I felt it more important yesterday
after reading a lot of files using things from tools/ but only
including them through other includes, which would break the code, if
I would remove the tools/ parts from the other includes first.


  // Let's put here misc structures that get passed to ScDocument's methods.
 diff --git a/sc/inc/drawpage.hxx b/sc/inc/drawpage.hxx
 index faa43fa..5e207b7 100644
 --- a/sc/inc/drawpage.hxx
 +++ b/sc/inc/drawpage.hxx
 @@ -30,7 +30,7 @@
  #define SC_DRAWPAGE_HXX

  #include svx/fmpage.hxx
 -
 +#include sal/types.h

  class ScDrawLayer;

 @@ -39,7 +39,7 @@ class ScDrawLayer;
  class ScDrawPage: public FmFormPage
  {
  public:
 -    ScDrawPage(ScDrawLayer rNewModel, StarBASIC* pBasic, BOOL 
 bMasterPage=FALSE);
 +    ScDrawPage(ScDrawLayer rNewModel, StarBASIC* pBasic, sal_Bool 
 bMasterPage=sal_False);
      ~ScDrawPage();

      virtual ::com::sun::star::uno::Reference 
 ::com::sun::star::uno::XInterface  createUnoPage();
 diff --git a/sc/source/core/data/drawpage.cxx 
 b/sc/source/core/data/drawpage.cxx
 index d489d5d..5a91540 100644
 --- a/sc/source/core/data/drawpage.cxx
 +++ b/sc/source/core/data/drawpage.cxx
 @@ -44,7 +44,7 @@

  // ---

 -ScDrawPage::ScDrawPage(ScDrawLayer rNewModel, StarBASIC* pBasic, BOOL 
 bMasterPage) :
 +ScDrawPage::ScDrawPage(ScDrawLayer rNewModel, StarBASIC* pBasic, sal_Bool 
 bMasterPage) :
      FmFormPage(rNewModel, pBasic, bMasterPage)
  {
      SetSize( Size( LONG_MAX, LONG_MAX ) );

 For these two, let's use the standard bool type.  In fact, its parent
 class FmFormPage does expect a parameter of the bool type, not sal_Bool
 nor BOOL (though the default value is for some reason set to sal_False
 instead of false, which should really be fixed...), which provides
 another reason to use bool, not sal_Bool.
I think I'm still too afraid of breaking UNO-things, if there just is
something UNO-related close to the code, when using native types, I
will try to be more confident in using bool, and believing in it, if
my build works out fine.

Regards
Sören



 Thanks!

 Kohei

 --
 Kohei Yoshida, LibreOffice hacker, Calc
 kyosh...@novell.com


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


Re: [Libreoffice] [PATCH] Removed dependencies on tools/solar.h

2011-01-04 Thread Soeren Moeller
Thank you for your replies, for the future I will use bool when
substituting BOOL outside of the UNO API.

With respect to ULONG, I, as Norbert correctly observed, used
sal_uIntPtr because of the typedef in tools/solar.h, but what should I
use instead? It seems there are the following possibilities:
- unsigned long
- sal_uInt32 (could be too short on 64-bit systems)
- sal_uIntPtr (wrong)
- C99 data types (but which of these, and as Norbert writes this would
require a compat.h for Microsoft compilers, and I have no idea how to
do this (and no Microsoft compiler to test it))

I would like to do similar removements of dependencies on tools/ in
other files in sc/inc/ (and the related source files), so
it would be nice to know. which data types I should use instead.

Regards
Sören


2011/1/4 Norbert Thiebaud nthieb...@gmail.com:
 On Mon, Jan 3, 2011 at 10:36 PM, Kohei Yoshida kyosh...@novell.com wrote:
 On Mon, 2011-01-03 at 21:47 +0100, Soeren Moeller wrote:
 Hi

 I have removed dependencies on tools/solar.h in some files in sc
 (according to 
 http://wiki.documentfoundation.org/Easy_Hacks#write_tools.2F_pieces_out
 ) please review and commit.

 Thanks, pushed!

 BTW, we generally prefer the standard bool over sal_Bool, so I replaced
 sal_Bool with bool in your patch.  The only place we need to use
 sal_Bool is when dealing with the UNO API.  Other than that, the
 standard boolean type is preferred.

 Also, it's a bit weird to use sal_uIntPtr which isn't used much in our
 code base.  So I replaced that with sal_uInt32.

 Kohei,

 I have not read the related code, but in principle uintptr_t and
 int32_t are not interchangeable.
  int32_t is 32 bit long, uintptr_t is supposed to be the same size
 than void* (that is 32 or 64 bits)

 in our sources,
 ULONG is typedef'ed as sal_uIntPrt (in tools/solar.h) , which is wrong (*)
 but that explain why Soeren used sal_uIntPtr.

 (*) it is wrong because there are multiple model of 64 bits support.
 Notoriously, Microsoft, as usual, instead of fixing their 64 bits
 support bugs, have, once again, turn their bugs into a standard and
 use the so-called LLP64 model, in which sizeof(long) != sizeof(void*)
 Note that ULONG is defined at multiple place, most of them as unsigned
 long (which conflict with the main definition of ULONG = sal_uIntPtr).
 Which raise the following question: has anyone successfully built
 LibreOffice for Win64 ?


 Note:
 C99 has been a standard for quite a while now. why are we not using
 the standardized type for these. that is:
 int8_t uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t,
 uint64_t, intptr_t, uintptr_t,...
 see http://en.wikipedia.org/wiki/Stdint.h

 Yes, I know, Microsoft still do not have a compiler compliant with the
 C-standard published 10 years ago... but that can be worked around
 with a compat.h header to hide Microsoft's screw-ups, without
 'uglyfying' the rest of the code.


 Kohei

 --
 Kohei Yoshida, LibreOffice hacker, Calc
 kyosh...@novell.com

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


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


[Libreoffice] [PATCH] Removed dependencies on tools/solar.h in waitoff.h

2011-01-04 Thread Soeren Moeller
Another small removal of dependencies on tools/solar.h, this time in
sc/inc/waitoff.h

Sören Möller
(LGPLv3+ / MPL)
From 64383ecc28b5c4aa09ecab31fb45be93ec4aa511 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6ren=20M=C3=B6ller?= soerenmoeller2...@gmail.com
Date: Tue, 4 Jan 2011 19:41:48 +0100
Subject: [PATCH] Removed dependencies on tools/solar.h in waitoff.h

Replaced datatypes from tools/solar.h in waitoff.h by corresponding types from sal/types.h
---
 sc/inc/waitoff.hxx |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sc/inc/waitoff.hxx b/sc/inc/waitoff.hxx
index d58f1cd..28d366e 100644
--- a/sc/inc/waitoff.hxx
+++ b/sc/inc/waitoff.hxx
@@ -29,7 +29,7 @@
 #ifndef SC_WAITOFF_HXX
 #define SC_WAITOFF_HXX
 
-#include tools/solar.h
+#include sal/types.h
 
 class Window;
 
@@ -37,7 +37,7 @@ class ScWaitCursorOff
 {
 private:
 Window*pWin;
-ULONGnWaiters;
+sal_uInt32			nWaiters;
 public:
 ScWaitCursorOff( Window* pWin );
 ~ScWaitCursorOff();
-- 
1.7.0.4

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