Re: [Libreoffice] [PATCH] Get rid of SvBytes/SvSvULongs in SwUndoDelNum

2011-02-08 Thread Nigel Hawkins
On Mon, 2011-02-07 at 23:31 +0300, Andrey Turkin wrote:
> So that would be the next step? Should I squash the two patches and resubmit 
> it?

Hopefully, Michael or somebody will push the pair of them.

Since we both seem to be looking at this section of the EasyHacks, can I
suggest we follow Michael's suggestion and add a note on the wiki that
we've started a bit. Then we can avoid duplicating effort.

Nigel.

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


Re: [Libreoffice] [PATCH] Get rid of SvBytes/SvSvULongs in SwUndoDelNum

2011-02-07 Thread Andrey Turkin
On Monday 07 February 2011 20:50:12 Nigel Hawkins wrote:
> On Mon, 2011-02-07 at 15:30 +, Michael Meeks wrote:
> > Perhaps you can build your patch on top of Andrey's to capture that
> > goodness ?
> 
> OK. Attached.
> 
So that would be the next step? Should I squash the two patches and resubmit 
it?
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] Get rid of SvBytes/SvSvULongs in SwUndoDelNum

2011-02-07 Thread Nigel Hawkins
On Mon, 2011-02-07 at 15:30 +, Michael Meeks wrote:
> Perhaps you can build your patch on top of Andrey's to capture that
> goodness ?

OK. Attached.

>  I'm busy adding a few more easy hacks to try to make such
> conflicts less likely: it can help to add your name and a date vs. an
> easy hack (with perhaps a sub-portion of what you're working on).

I shall try and remember to do this in future.

Nigel
>From 00f1b9804c1f19c1bb7e6c790910f9005f89321b Mon Sep 17 00:00:00 2001
From: Nigel Hawkins 
Date: Mon, 7 Feb 2011 16:19:59 +
Subject: [PATCH] Replace std::pair with more descriptive names in SwUndoDelNum

---
 sw/inc/undobj.hxx |   10 +++---
 sw/source/core/undo/unnum.cxx |   10 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index e6ea274..ae86015 100644
--- a/sw/inc/undobj.hxx
+++ b/sw/inc/undobj.hxx
@@ -1371,9 +1371,13 @@ public:
 
 class SwUndoDelNum : public SwUndo, private SwUndRng
 {
-typedef std::vector< std::pair< ULONG, int > > SvNode;
-SvNode aNodes;
-SvBools aRstLRSpaces;
+struct NodeLevel
+{
+ULONG index;
+int level;
+inline NodeLevel(ULONG idx, int lvl) : index(idx), level(lvl) {};
+};
+std::vector aNodes;
 SwHistory* pHistory;
 public:
 SwUndoDelNum( const SwPaM& rPam );
diff --git a/sw/source/core/undo/unnum.cxx b/sw/source/core/undo/unnum.cxx
index d5afd1f..f13f94c 100644
--- a/sw/source/core/undo/unnum.cxx
+++ b/sw/source/core/undo/unnum.cxx
@@ -214,11 +214,11 @@ void SwUndoDelNum::Undo( SwUndoIter& rUndoIter )
 pHistory->TmpRollback( &rDoc, 0 );
 pHistory->SetTmpEnd( pHistory->Count() );
 
-for( SvNode::const_iterator i = aNodes.begin(); i != aNodes.end(); ++i )
+for( std::vector::const_iterator i = aNodes.begin(); i != aNodes.end(); ++i )
 {
-SwTxtNode* pNd = rDoc.GetNodes()[ i->first ]->GetTxtNode();
-OSL_ENSURE( pNd, "wo ist der TextNode geblieben?" );
-pNd->SetAttrListLevel( i->second );
+SwTxtNode* pNd = rDoc.GetNodes()[ i->index ]->GetTxtNode();
+OSL_ENSURE( pNd, "Where has the TextNode gone?" );
+pNd->SetAttrListLevel( i->level );
 
 if( pNd->GetCondFmtColl() )
 pNd->ChkCondColl();
@@ -246,7 +246,7 @@ void SwUndoDelNum::AddNode( const SwTxtNode& rNd, BOOL )
 {
 if( rNd.GetNumRule() )
 {
-aNodes.push_back( SvNode::value_type( rNd.GetIndex(), rNd.GetActualListLevel() ) );
+aNodes.push_back( NodeLevel( rNd.GetIndex(), rNd.GetActualListLevel() ) );
 }
 }
 
-- 
1.7.0.4

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


Re: [Libreoffice] [PATCH] Get rid of SvBytes/SvSvULongs in SwUndoDelNum

2011-02-07 Thread Michael Meeks
Hi Nigel,

On Mon, 2011-02-07 at 14:33 +, Nigel Hawkins wrote:
> Oddly enough, I was looking at this the other day but didn't get round
> to submitting a patch because I couldn't get sw to compile (for totally
> unrelated reasons). My changes were almost identical to yours, though I
> have one suggestion.

Drat; sorry to hear work got duplicated.

> Changing to a std::pair means we go from descriptively named items
> (aNodeIdx and aLevels) to the generic first/second. Which is less clear
> to the reader. Can I suggest you either:

Perhaps you can build your patch on top of Andrey's to capture that
goodness ? I'm busy adding a few more easy hacks to try to make such
conflicts less likely: it can help to add your name and a date vs. an
easy hack (with perhaps a sub-portion of what you're working on).

ATB,

Michael.

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


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


Re: [Libreoffice] [PATCH] Get rid of SvBytes/SvSvULongs in SwUndoDelNum

2011-02-07 Thread Nigel Hawkins
Hi Andrey,

> This is my first patch to LibreOffice so I'd be grateful for feedback on any 
> issues.

Oddly enough, I was looking at this the other day but didn't get round
to submitting a patch because I couldn't get sw to compile (for totally
unrelated reasons). My changes were almost identical to yours, though I
have one suggestion.

Changing to a std::pair means we go from descriptively named items
(aNodeIdx and aLevels) to the generic first/second. Which is less clear
to the reader. Can I suggest you either:

A) Add a comment above the SvNode definition to say (very briefly) what
the ULONG and int are used for.

or:

B) Define a local class/struct instead of the pair with more descriptive
names for the elements.

Nigel

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


[Libreoffice] [PATCH] Get rid of SvBytes/SvSvULongs in SwUndoDelNum

2011-02-07 Thread Andrey Turkin
Small part of one of tasks in "easy tasks" list.

This is my first patch to LibreOffice so I'd be grateful for feedback on any 
issues.

Released under LGPLv3+/MPL

---
 sw/inc/undobj.hxx |4 ++--
 sw/source/core/undo/unnum.cxx |   16 ++--
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx
index 2252de9..e6ea274 100644
--- a/sw/inc/undobj.hxx
+++ b/sw/inc/undobj.hxx
@@ -1371,8 +1371,8 @@ public:
 
 class SwUndoDelNum : public SwUndo, private SwUndRng
 {
-SvULongs aNodeIdx;
-SvBytes aLevels;
+typedef std::vector< std::pair< ULONG, int > > SvNode;
+SvNode aNodes;
 SvBools aRstLRSpaces;
 SwHistory* pHistory;
 public:
diff --git a/sw/source/core/undo/unnum.cxx b/sw/source/core/undo/unnum.cxx
index bc2be15..d5afd1f 100644
--- a/sw/source/core/undo/unnum.cxx
+++ b/sw/source/core/undo/unnum.cxx
@@ -190,10 +190,9 @@ void SwUndoInsNum::SaveOldNumRule( const SwNumRule& rOld )
 
 
 SwUndoDelNum::SwUndoDelNum( const SwPaM& rPam )
-: SwUndo( UNDO_DELNUM ), SwUndRng( rPam ),
-aNodeIdx( BYTE( nEndNode - nSttNode > 255 ? 255 : nEndNode - nSttNode )),
-aLevels( BYTE( nEndNode - nSttNode > 255 ? 255 : nEndNode - nSttNode ))
+: SwUndo( UNDO_DELNUM ), SwUndRng( rPam )
 {
+aNodes.reserve( nEndNode - nSttNode > 255 ? 255 : nEndNode - nSttNode );
 pHistory = new SwHistory;
 }
 
@@ -215,11 +214,11 @@ void SwUndoDelNum::Undo( SwUndoIter& rUndoIter )
 pHistory->TmpRollback( &rDoc, 0 );
 pHistory->SetTmpEnd( pHistory->Count() );
 
-for( USHORT n = 0; n < aNodeIdx.Count(); ++n )
+for( SvNode::const_iterator i = aNodes.begin(); i != aNodes.end(); ++i )
 {
-SwTxtNode* pNd = rDoc.GetNodes()[ aNodeIdx[ n ] ]->GetTxtNode();
+SwTxtNode* pNd = rDoc.GetNodes()[ i->first ]->GetTxtNode();
 OSL_ENSURE( pNd, "wo ist der TextNode geblieben?" );
-pNd->SetAttrListLevel(aLevels[ n ] );
+pNd->SetAttrListLevel( i->second );
 
 if( pNd->GetCondFmtColl() )
 pNd->ChkCondColl();
@@ -247,10 +246,7 @@ void SwUndoDelNum::AddNode( const SwTxtNode& rNd, BOOL )
 {
 if( rNd.GetNumRule() )
 {
-USHORT nIns = aNodeIdx.Count();
-aNodeIdx.Insert( rNd.GetIndex(), nIns );
-
-aLevels.Insert( static_cast(rNd.GetActualListLevel()), nIns );
+aNodes.push_back( SvNode::value_type( rNd.GetIndex(), rNd.GetActualListLevel() ) );
 }
 }
 
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice