[REVIEW-3-6 3-6-2] several patches for bugs in conditional formats/ ScRangeList

Hey,

I have several patches that are needed to fix bugs in conditional
formats. One of them is a MAB/regression with several duplicates.

They apply cleanly on 3-6 and you may squash them into one large
commit but it makes reviewing them even more complicated. I did not do
that yet because it makes reviewing even more difficult. They allow to
update ScRangeLists correctly when the cells are moved or deleted.
There is one small problem with updating moving only parts of one of
the ScRange entries but this will require more concept work to find
all corner cases and to implement this. These cases are not that
important and these patches here are already an improvement in this
direction.

Please apply them in the following order:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7222a571d0d458810c1b23871f8b91491db4462d
http://cgit.freedesktop.org/libreoffice/core/commit/?id=a3c4ee1653166ee2ac1f1b9d65ff1065b6288ebc
http://cgit.freedesktop.org/libreoffice/core/commit/?id=4cf0759e7c6bd698c929a11c771d2ab03f1b9536
http://cgit.freedesktop.org/libreoffice/core/commit/?id=e6bca122176cdb2b6e822fc933f159dc3e3c8d46
http://cgit.freedesktop.org/libreoffice/core/commit/?id=7a182026fce922a9f69e8da76d46e87e7188a4e9
http://cgit.freedesktop.org/libreoffice/core/commit/?id=764e7e71038d5ae66061f44bc0cd51ce33ae96ed
http://cgit.freedesktop.org/libreoffice/core/commit/?id=1e3919f040ade5d0f7f9fa854b3ed23366080c0c

Nearly all of these patches are only changing ScRangeList and not
directly the conditional format code. I have some unit tests for
ScRangeList::UpdateReference and ScRangeList::DeleteArea in
sc/qa/unit/rangelst_test.cxx

Regards,
Markus
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

On 09/21/2012 08:12 AM, Markus Mohrhard wrote:
I've been reviewing these patches since this morning, and I'm only
half-way through.  I'll need the rest of the day to fully review this patch.

Though I try to approve this change into 3.6, given the size of the
change and how close we are to release 3.6.2, I'd be more comfortable if
we targeted this for 3.6.3, to give us more time to review.

Best,

Kohei

--
Kohei Yoshida, LibreOffice hacker, Calc
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

On 09/21/2012 01:30 PM, Kohei Yoshida wrote:
> On 09/21/2012 08:12 AM, Markus Mohrhard wrote:
I've squashed all these plus

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0e1e59057d005c9333a49ce7b2ae949a3121c55e

into a single commit.  All these commits are from Markus, and I have
signed-off on this, on the condition that we also backport my own change
on top of it to fix several issues that I discovered and fixed during my
review.

The attached 0001 patch is Markus' patch, and the 0002 patch is mine on
top of it.  Mine is basically a backport of

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5551cd0209981f71ea5fb252b791391a6427066e

and

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cb7ee824dc0b9dcc2fd466f190945de01a9d1fa5

minus the unit test piece which doesn't exist in the 3-6 branch.

Now, technically someone has to sign-off on my proposed change on top of
Markus, so whoever signs off on it will have to first commit Markus'
patch with my sign-off, and then commit mine.

Regards,

Kohei

--
Kohei Yoshida, LibreOffice hacker, Calc

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Hey,

2012/9/22 Kohei Yoshida <[hidden email]>:
Thanks a lot for reviewing these. While reviewing Kohei's changes I
found 6 more cases of < where <= would be correct.

So please also cherry-pick
http://cgit.freedesktop.org/libreoffice/core/commit/?id=925ed0b79bc400a72eaaf7c8b53b67d96c7cab7a

The corresponding tests for these changes have been added with
http://cgit.freedesktop.org/libreoffice/core/commit/?id=fc0aa44b9d6aab7af68b00e4e26f3d9300e30fc2

I also agree with Kohei that after we found these problems it is
better to push this only to 3-6 and I will send an ugly fix for the
crash that will just open one of the other fixed bugs but will prevent
the crash in 3.6.2.

And just for the record: Kohei's changes to my original patch look
good, I'm not sure if I can give my sign-off on them.

Regards,
Markus
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Hi Markus,

On 2012-09-22 at 05:00 +0200, Markus Mohrhard wrote:

> And just for the record: Kohei's changes to my original patch look
> good, I'm not sure if I can give my sign-off on them.

Works for me, please go ahead :-)

All the best,
Kendy

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice

All three patches pushed to 3-6.

Kohei
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice