On Wed, 2011-11-23 at 18:32 +0900, Daisuke Nishino wrote:
> Hi,
>
> This patch makes SwSelBoxes derived from std::map, getting rid of use
> of SV_DECL_PTRARR_SORT.
> The patch is available under LGPLv3+/MPL.
Phew, this is a real big patch. Looks ok to me, though always tricky to
get these conversions right. Hopefully nothing slipped past me. Here's
some thoughts as I went through it.
a) // dann die Positionen der neuen Size anpasse hunk
I initially wondered why this block got removed, but looking at the
code in place where there's more context I see
aPosArr.clear();
...
// dann die Positionen der neuen Size anpassen
for( n = 0; n < aPosArr.size(); ++n )
{
...
}
i.e. aPosArr always empty, so block never run, so your change looks
good. But looking backwards with git blame, I see that in a0a1c3f4
- aPosArr.Remove( 0, n );
+ aPosArr.clear();
and looking where n came from...
for( n = 0; n < aPosArr.size(); ++n )
if( aPosArr[ n ] == nOffset )
break;
else if( aPosArr[ n ] > nOffset )
{
if( n )
--n;
break;
}
so that *prior* change (nothing to do with you) wasn't right IMO. So
changed that code and dropped your hunk which was based off aPosArr
being always empty. Looks like we found and fixed a bug in a previous
round of conversions.
b) SwSelBoxes_SAR is now a std::vector, so
- aBoxes.Remove( 0, n );
+ for( sal_uInt16 i = 0; i < n; ++i )
+ aBoxes.erase( aBoxes.begin() );
can just be
- aBoxes.Remove( 0, n );
+ aBoxes.erase(aBoxes.begin(), aBoxes.begin()+n);
right ?
Please have a look at my changes in sw that differ from the original
patch to see if they make sense.
C.
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
