Date: prev next · Thread: first prev next last
2011 Archives by date, by thread · List index


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.


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.