Re: [Libreoffice] [PATCH] Get rid of SvULongs in calc
On Tue, Jul 19, 2011 at 11:06:49AM +0200, Maciej Rumianowski wrote: Hi Kohei, 4) Last but not least, please state that you are submitting your patches under LGPLv3+/MPL 1.1. Of course and future also. Hi, could you add yourself to http://wiki.documentfoundation.org/Development/Developers , please? D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Get rid of SvULongs in calc
On Tue, 2011-07-19 at 11:06 +0200, Maciej Rumianowski wrote: Hi Kohei, Hi Maciej, So, I've reviewed your revised patches and they look good. I've pushed them to master with one minor syntactic change. I've changed in ScTabViewShell::Execute() if ( aIndexList.size() ) to if ( !aIndexList.empty() ) For vector both are fine performance-wise though for other STL containers using empty() is faster. But even with vector it's better to make this a habit IMO. Anyway, thanks for your patches again, and we look forward to receiving more patches from you in the future. :-) 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] Get rid of SvULongs in calc
Dnia 2011-07-20, śro o godzinie 17:12 -0400, Kohei Yoshida pisze: So, I've reviewed your revised patches and they look good. I've pushed them to master with one minor syntactic change. Great thanks :) Anyway, thanks for your patches again, and we look forward to receiving more patches from you in the future. :-) I'm working further on Bug 38831 - [EasyHack] Get rid of SV_DECL_VARARR, SV_DECL_VARARR_PLAIN, SV_DECL_VARARR_SORT ... Where can I mark that I'm working on it? In Bug comments? Best Regards, Maciej ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Get rid of SvULongs in calc
Hi Maciej, On Wed, Jul 20, 2011 at 5:24 PM, Maciej Rumianowski maciej.rumianow...@gmail.com wrote: I'm working further on Bug 38831 - [EasyHack] Get rid of SV_DECL_VARARR, SV_DECL_VARARR_PLAIN, SV_DECL_VARARR_SORT ... Where can I mark that I'm working on it? In Bug comments? Well, you can probably just mention that in the Easy Hacks wiki page since this task is featured on that page. Also, let Nigel know which one you are tackling since he is also working on this task as well. For other, non-featured tasks I would recommend just mention that in their respective bugzilla page and/or assign the task to yourself. Best, Kohei ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Get rid of SvULongs in calc
Hi Kohei,
Thanks for review!
First, for your svl patch:
1) in SfxIntegerListItem::GetList(), you do
+rList.insert( aIt, m_aList[n] );
but you can simply call push_back here to append new data to the end of
the vector. Also, let's use pre-increment on iterators in the for
loop there i.e. instead of doing aIt++, do ++aIt. Post-increment on
iterators is generally more expensive than pre-increment, so it's best
to stick with pre-increments.
Done, and with push_back we don't need iterator.
2) Your new constructors take arrays of different integer type; one
takes std::vectorsal_uLong while the other takes
uno::Sequencesal_Int32. Let's just stick with one type and use
sal_Int32 in both cases since that's the integer type that this class
uses internally.
Done and checked in other places to use right type.
And for your sc patch:
3) In ScTabViewShell::Execute() you do
-SvULongs aIndexList( 4, 4 );
+::std::vector sal_uLong aIndexList( 4, 4 );
but this actually changes the behavior of the code. The arguments to
SvULong's constructor controls how the internal array is laid out during
initialization but it doesn't populate the container with data.
Vector's ctor arguments OTOH populate the container with data. So, your
new code initializes aIndexList with 4 elements of value 4, which is not
what the code intends to do. You can simply do without passing any
arguments to aIndexList there.
Done, Now I understand the SvULong's constructor. Also changed type to
sal_uInt32.
4) Last but not least, please state that you are submitting your patches
under LGPLv3+/MPL 1.1.
Of course and future also.
Best Regards,
Maciej
From a4f2da5241afdd8062f9611ca1f580df1c2ffe54 Mon Sep 17 00:00:00 2001
From: Maciej Rumianowski maciej.rumianow...@gmail.com
Date: Tue, 19 Jul 2011 10:50:54 +0200
Subject: [PATCH] Get rid of SvULongs in calc
Instead of SvULongs use ::std::vector sal_Int32
---
.../itemsetwrapper/DataPointItemConverter.cxx |7 +--
.../itemsetwrapper/SeriesOptionsItemConverter.cxx |7 +--
chart2/source/view/main/ChartItemPool.cxx |8 +++-
sc/source/ui/docshell/impex.cxx| 11 +--
sc/source/ui/view/tabvwsh3.cxx | 11 +--
5 files changed, 15 insertions(+), 29 deletions(-)
diff --git a/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx b/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
index 3802ffc..76e7db0 100644
--- a/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
+++ b/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
@@ -54,8 +54,6 @@
#include editeng/brshitem.hxx
//SfxIntegerListItem
#include svl/ilstitem.hxx
-#define _SVSTDARR_ULONGS
-#include svl/svstdarr.hxx
#include vcl/graph.hxx
#include com/sun/star/graphic/XGraphic.hpp
@@ -641,10 +639,7 @@ void DataPointItemConverter::FillSpecialItem(
case SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS:
{
-SvULongs aList;
-for ( sal_Int32 nN=0; nNm_aAvailableLabelPlacements.getLength(); nN++ )
-aList.Insert( m_aAvailableLabelPlacements[nN], sal::static_int_cast sal_uInt16 (nN) );
-rOutItemSet.Put( SfxIntegerListItem( nWhichId, aList ) );
+rOutItemSet.Put( SfxIntegerListItem( nWhichId, m_aAvailableLabelPlacements ) );
}
break;
diff --git a/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx b/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
index a199b66..7df2d61 100644
--- a/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
+++ b/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
@@ -51,8 +51,6 @@
//SfxIntegerListItem
#include svl/ilstitem.hxx
-#define _SVSTDARR_ULONGS
-#include svl/svstdarr.hxx
#include rtl/math.hxx
#include functional
@@ -433,10 +431,7 @@ void SeriesOptionsItemConverter::FillSpecialItem(
}
case SCHATTR_AVAILABLE_MISSING_VALUE_TREATMENTS:
{
-SvULongs aList;
-for ( sal_Int32 nN=0; nNm_aSupportedMissingValueTreatments.getLength(); nN++ )
-aList.Insert( m_aSupportedMissingValueTreatments[nN], sal::static_int_cast sal_uInt16 (nN) );
-rOutItemSet.Put( SfxIntegerListItem( nWhichId, aList ) );
+rOutItemSet.Put( SfxIntegerListItem( nWhichId, m_aSupportedMissingValueTreatments ) );
break;
}
case SCHATTR_INCLUDE_HIDDEN_CELLS:
diff --git a/chart2/source/view/main/ChartItemPool.cxx b/chart2/source/view/main/ChartItemPool.cxx
index 2cdac8f..aa81e06 100644
--- a/chart2/source/view/main/ChartItemPool.cxx
+++ b/chart2/source/view/main/ChartItemPool.cxx
@@ -39,10 +39,9 @@
#include svl/stritem.hxx
#include svl/rectitem.hxx
#include svl/ilstitem.hxx
-#define _SVSTDARR_ULONGS
-#include svl/svstdarr.hxx
#include
Re: [Libreoffice] [PATCH] Get rid of SvULongs in calc
Spotted some mistakes in calc patch.
From e7db01344f8fef4ca2f2f59868736033d0126b1b Mon Sep 17 00:00:00 2001
From: Maciej Rumianowski maciej.rumianow...@gmail.com
Date: Mon, 18 Jul 2011 09:22:57 +0200
Subject: [PATCH] Get rid of SvULongs in calc
Instead of SvULongs use ::std::vector sal_uLong
---
.../itemsetwrapper/DataPointItemConverter.cxx |7 +--
.../itemsetwrapper/SeriesOptionsItemConverter.cxx |7 +--
chart2/source/view/main/ChartItemPool.cxx |8 +++-
sc/source/ui/docshell/impex.cxx| 11 +--
sc/source/ui/view/tabvwsh3.cxx |8
5 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx b/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
index 3802ffc..76e7db0 100644
--- a/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
+++ b/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
@@ -54,8 +54,6 @@
#include editeng/brshitem.hxx
//SfxIntegerListItem
#include svl/ilstitem.hxx
-#define _SVSTDARR_ULONGS
-#include svl/svstdarr.hxx
#include vcl/graph.hxx
#include com/sun/star/graphic/XGraphic.hpp
@@ -641,10 +639,7 @@ void DataPointItemConverter::FillSpecialItem(
case SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS:
{
-SvULongs aList;
-for ( sal_Int32 nN=0; nNm_aAvailableLabelPlacements.getLength(); nN++ )
-aList.Insert( m_aAvailableLabelPlacements[nN], sal::static_int_cast sal_uInt16 (nN) );
-rOutItemSet.Put( SfxIntegerListItem( nWhichId, aList ) );
+rOutItemSet.Put( SfxIntegerListItem( nWhichId, m_aAvailableLabelPlacements ) );
}
break;
diff --git a/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx b/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
index a199b66..7df2d61 100644
--- a/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
+++ b/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
@@ -51,8 +51,6 @@
//SfxIntegerListItem
#include svl/ilstitem.hxx
-#define _SVSTDARR_ULONGS
-#include svl/svstdarr.hxx
#include rtl/math.hxx
#include functional
@@ -433,10 +431,7 @@ void SeriesOptionsItemConverter::FillSpecialItem(
}
case SCHATTR_AVAILABLE_MISSING_VALUE_TREATMENTS:
{
-SvULongs aList;
-for ( sal_Int32 nN=0; nNm_aSupportedMissingValueTreatments.getLength(); nN++ )
-aList.Insert( m_aSupportedMissingValueTreatments[nN], sal::static_int_cast sal_uInt16 (nN) );
-rOutItemSet.Put( SfxIntegerListItem( nWhichId, aList ) );
+rOutItemSet.Put( SfxIntegerListItem( nWhichId, m_aSupportedMissingValueTreatments ) );
break;
}
case SCHATTR_INCLUDE_HIDDEN_CELLS:
diff --git a/chart2/source/view/main/ChartItemPool.cxx b/chart2/source/view/main/ChartItemPool.cxx
index 2cdac8f..82df33d 100644
--- a/chart2/source/view/main/ChartItemPool.cxx
+++ b/chart2/source/view/main/ChartItemPool.cxx
@@ -39,10 +39,9 @@
#include svl/stritem.hxx
#include svl/rectitem.hxx
#include svl/ilstitem.hxx
-#define _SVSTDARR_ULONGS
-#include svl/svstdarr.hxx
#include editeng/editids.hrc
#include svx/svxids.hrc
+#include vector
#include com/sun/star/chart2/LegendPosition.hpp
@@ -63,8 +62,7 @@ ChartItemPool::ChartItemPool():
ppPoolDefaults[SCHATTR_DATADESCR_SHOW_SYMBOL- SCHATTR_START] = new SfxBoolItem(SCHATTR_DATADESCR_SHOW_SYMBOL);
ppPoolDefaults[SCHATTR_DATADESCR_SEPARATOR - SCHATTR_START] = new SfxStringItem(SCHATTR_DATADESCR_SEPARATOR,C2U( ));
ppPoolDefaults[SCHATTR_DATADESCR_PLACEMENT - SCHATTR_START] = new SfxInt32Item(SCHATTR_DATADESCR_PLACEMENT,0);
-SvULongs aTmp;
-ppPoolDefaults[SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS - SCHATTR_START] = new SfxIntegerListItem(SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS,aTmp);
+ppPoolDefaults[SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS - SCHATTR_START] = new SfxIntegerListItem(SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS, ::std::vector sal_uLong () );
ppPoolDefaults[SCHATTR_DATADESCR_NO_PERCENTVALUE- SCHATTR_START] = new SfxBoolItem(SCHATTR_DATADESCR_NO_PERCENTVALUE);
ppPoolDefaults[SCHATTR_PERCENT_NUMBERFORMAT_VALUE - SCHATTR_START] = new SfxUInt32Item(SCHATTR_PERCENT_NUMBERFORMAT_VALUE, 0);
ppPoolDefaults[SCHATTR_PERCENT_NUMBERFORMAT_SOURCE - SCHATTR_START] = new SfxBoolItem(SCHATTR_PERCENT_NUMBERFORMAT_SOURCE);
@@ -157,7 +155,7 @@ ChartItemPool::ChartItemPool():
ppPoolDefaults[SCHATTR_CLOCKWISE- SCHATTR_START] = new SfxBoolItem( SCHATTR_CLOCKWISE, sal_False );
ppPoolDefaults[SCHATTR_MISSING_VALUE_TREATMENT- SCHATTR_START] = new SfxInt32Item(SCHATTR_MISSING_VALUE_TREATMENT, 0);
-ppPoolDefaults[SCHATTR_AVAILABLE_MISSING_VALUE_TREATMENTS - SCHATTR_START] = new
Re: [Libreoffice] [PATCH] Get rid of SvULongs in calc
Hi Maciej, First of all, thanks for submitting your patches. I've done my review of both of your patches, and I'll leave my comments below. First, for your svl patch: 1) in SfxIntegerListItem::GetList(), you do +rList.insert( aIt, m_aList[n] ); but you can simply call push_back here to append new data to the end of the vector. Also, let's use pre-increment on iterators in the for loop there i.e. instead of doing aIt++, do ++aIt. Post-increment on iterators is generally more expensive than pre-increment, so it's best to stick with pre-increments. 2) Your new constructors take arrays of different integer type; one takes std::vectorsal_uLong while the other takes uno::Sequencesal_Int32. Let's just stick with one type and use sal_Int32 in both cases since that's the integer type that this class uses internally. And for your sc patch: 3) In ScTabViewShell::Execute() you do -SvULongs aIndexList( 4, 4 ); +::std::vector sal_uLong aIndexList( 4, 4 ); but this actually changes the behavior of the code. The arguments to SvULong's constructor controls how the internal array is laid out during initialization but it doesn't populate the container with data. Vector's ctor arguments OTOH populate the container with data. So, your new code initializes aIndexList with 4 elements of value 4, which is not what the code intends to do. You can simply do without passing any arguments to aIndexList there. 4) Last but not least, please state that you are submitting your patches under LGPLv3+/MPL 1.1. That's all I can think of. Let's get these points revised, and I'll push the changes to master. Best, 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] Get rid of SvULongs in calc
Hi,
I'm sending a Patch which converts SvULongs to std::vector sal_uLong
.
I would like to add a constructor
SfxIntegerListItem::SfxIntegerListItem( const ::com::sun::star::uno::Sequence
sal_Int32 rList ), because DataPointItemConverter and
SeriesOptionsItemConverter are copying from Sequence to SvULongs and then in
SfxIntegerListItem SvLongs is copied to Sequence.
As I wrote before I have added a constructor to SfxIntegerListItem, is
it okay?
Best Regards,
Maciej
From 6af35271b8738116393900ba349354d1f3f0bf1b Mon Sep 17 00:00:00 2001
From: Maciej Rumianowski maciej.rumianow...@gmail.com
Date: Sun, 17 Jul 2011 23:24:45 +0200
Subject: [PATCH] Get rid of SvULongs in calc
Instead of SvULongs use ::std::vector sal_uLong
---
.../itemsetwrapper/DataPointItemConverter.cxx |6 +++---
.../itemsetwrapper/SeriesOptionsItemConverter.cxx |6 +++---
chart2/source/view/main/ChartItemPool.cxx |5 ++---
sc/source/ui/docshell/impex.cxx| 11 +--
sc/source/ui/view/tabvwsh3.cxx |8
5 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx b/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
index 3802ffc..fb1c5f8 100644
--- a/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
+++ b/chart2/source/controller/itemsetwrapper/DataPointItemConverter.cxx
@@ -641,10 +641,10 @@ void DataPointItemConverter::FillSpecialItem(
case SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS:
{
-SvULongs aList;
+/*SvULongs aList;
for ( sal_Int32 nN=0; nNm_aAvailableLabelPlacements.getLength(); nN++ )
-aList.Insert( m_aAvailableLabelPlacements[nN], sal::static_int_cast sal_uInt16 (nN) );
-rOutItemSet.Put( SfxIntegerListItem( nWhichId, aList ) );
+aList.Insert( m_aAvailableLabelPlacements[nN], sal::static_int_cast sal_uInt16 (nN) );*/
+rOutItemSet.Put( SfxIntegerListItem( nWhichId, m_aAvailableLabelPlacements ) );
}
break;
diff --git a/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx b/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
index a199b66..dd844c6 100644
--- a/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
+++ b/chart2/source/controller/itemsetwrapper/SeriesOptionsItemConverter.cxx
@@ -433,10 +433,10 @@ void SeriesOptionsItemConverter::FillSpecialItem(
}
case SCHATTR_AVAILABLE_MISSING_VALUE_TREATMENTS:
{
-SvULongs aList;
+/*SvULongs aList;
for ( sal_Int32 nN=0; nNm_aSupportedMissingValueTreatments.getLength(); nN++ )
-aList.Insert( m_aSupportedMissingValueTreatments[nN], sal::static_int_cast sal_uInt16 (nN) );
-rOutItemSet.Put( SfxIntegerListItem( nWhichId, aList ) );
+aList.Insert( m_aSupportedMissingValueTreatments[nN], sal::static_int_cast sal_uInt16 (nN) );*/
+rOutItemSet.Put( SfxIntegerListItem( nWhichId, m_aSupportedMissingValueTreatments ) );
break;
}
case SCHATTR_INCLUDE_HIDDEN_CELLS:
diff --git a/chart2/source/view/main/ChartItemPool.cxx b/chart2/source/view/main/ChartItemPool.cxx
index 2cdac8f..da10db9 100644
--- a/chart2/source/view/main/ChartItemPool.cxx
+++ b/chart2/source/view/main/ChartItemPool.cxx
@@ -63,8 +63,7 @@ ChartItemPool::ChartItemPool():
ppPoolDefaults[SCHATTR_DATADESCR_SHOW_SYMBOL- SCHATTR_START] = new SfxBoolItem(SCHATTR_DATADESCR_SHOW_SYMBOL);
ppPoolDefaults[SCHATTR_DATADESCR_SEPARATOR - SCHATTR_START] = new SfxStringItem(SCHATTR_DATADESCR_SEPARATOR,C2U( ));
ppPoolDefaults[SCHATTR_DATADESCR_PLACEMENT - SCHATTR_START] = new SfxInt32Item(SCHATTR_DATADESCR_PLACEMENT,0);
-SvULongs aTmp;
-ppPoolDefaults[SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS - SCHATTR_START] = new SfxIntegerListItem(SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS,aTmp);
+ppPoolDefaults[SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS - SCHATTR_START] = new SfxIntegerListItem(SCHATTR_DATADESCR_AVAILABLE_PLACEMENTS, ::std::vector sal_uLong () );
ppPoolDefaults[SCHATTR_DATADESCR_NO_PERCENTVALUE- SCHATTR_START] = new SfxBoolItem(SCHATTR_DATADESCR_NO_PERCENTVALUE);
ppPoolDefaults[SCHATTR_PERCENT_NUMBERFORMAT_VALUE - SCHATTR_START] = new SfxUInt32Item(SCHATTR_PERCENT_NUMBERFORMAT_VALUE, 0);
ppPoolDefaults[SCHATTR_PERCENT_NUMBERFORMAT_SOURCE - SCHATTR_START] = new SfxBoolItem(SCHATTR_PERCENT_NUMBERFORMAT_SOURCE);
@@ -157,7 +156,7 @@ ChartItemPool::ChartItemPool():
ppPoolDefaults[SCHATTR_CLOCKWISE- SCHATTR_START] = new SfxBoolItem( SCHATTR_CLOCKWISE, sal_False );
ppPoolDefaults[SCHATTR_MISSING_VALUE_TREATMENT- SCHATTR_START] = new SfxInt32Item(SCHATTR_MISSING_VALUE_TREATMENT, 0);
-
