Re: [Libreoffice] [PATCH] fdo 42286 in calc solved ?

2011-11-16 Thread Kohei Yoshida
On Tue, 2011-11-15 at 21:12 +0100, Pierre-André Jacquod wrote:

 My solution: suppress this call...
 I have tested it and it seems to work. I did not managed to find a case 
 where something is broken with the filter..
 
 But it seems me too simple.

Yes it is too simple, isn't it?

The good news is that, you are in the right neighborhood.  The bad news
is that, you've just removed a valid functionality.  I'm afraid I can't
push your change, since the line you wish to remove is there for a
reason.

Also, your use case is a weird one.  Normally Calc users don't want to
filter out the bottom empty rows.  In fact, if you have an empty row it
actually acts as a data range separator, which affects what range is
used for so-called database operations; filtering, subtotal, sorting
etc.  That's an intended feature, not a bug, and supporting use cases
like yours will not be easy  require non-trivial code change and
perhaps several configuration options too (since the default behavior
can't possibly please all users).

Having said that, this auto range selection code for database ranges has
grown too complex  probably needs some cleanup.  But I'm afraid
removing that line will not be the way to go.  Sorry.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc

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


Re: [Libreoffice] [PATCH] fdo 42286 in calc solved ?

2011-11-15 Thread Pierre-André Jacquod

Hello,

Thanks for your hints. It took some times, but know I have clearly 
identified the problem , but I do not if my solution is right or could 
create more problems.


In short, the problem is that GetAreadData is called each time the 
filter determines its values, instead of using the current area. This 
does not have influence, as long as your selection area does not end 
with empties rows or empties columns.


But this behaviour produce a wrong result as soon as your filtered area 
ends with empties rows, and as special case, if you select all rows, 
since it decreases the area of 1 row if the last row is empty.


Basically this happens with the call of function ExtendDataArea. This 
originates from documen3.cxx, function GetFilterEntries line 1390 (the 
call itself is at line 1398). (more details below, if needed)


My solution: suppress this call...
I have tested it and it seems to work. I did not managed to find a case 
where something is broken with the filter..


But it seems me too simple.
Could you check this before I push it ?

If Ok, would you like then to approve it  cherry-pick it to 3.4.5 ?

Thanks  best regards
Pierre-André

ps: if what I did is right, I will then update the bug ticket

---

What happens?
As I understand after debugging and following the logic, each time the 
filter is activated (it is an auto-filter) it tries do determine the 
data range when it search for its value to show in the filter drop-down.


For that it searches the biggest contiguous area of data, as when 
applying the filter without selection.


Except the fact that here it goes wrong, since an area has already 
selected (either manually or stored as table:database-range ... tag.
which contains and finish (important in this analysis) with several 
empty rows (columns would produce the same problem). And it tries to fit 
the Area with the GetAreaData algorithm, starting with a predefined 
area. The result: it does not extend it (next row is emtpy), but 
thinking he as extended the area of one row to far (this the last row 
int the area is empty), it reduce the selected area of one row.


And this produce the strange comportment described. (more detailed 
analysis below).


Each time the filter value is changed , the function 
ScTAble::GetAreaData (in  sc/source/core/data/table1.cxx, line 740 in my 
check-out) is called.
At the end of the called function, there is a check: if the last row is 
empty, then the Area is reduced by one, deacreasing its last row value 
by 1 (line 835/836)

if (!bFound)
--rEndRow;

This is obviously wrong, especially if you select e.g the 3 first 
columns (completely) and then apply auto-filter. Since empty lines *are* 
part of the selection... and not only the last one is empty. So each 
time I change the filter value, I reduce by 1 the last row number...



I see that the content.xml ends with some strange empty row

It is ill-formatted by the user ;-)  Cells in columns A:G down to the



However, strange are the spurious entries with
table:visibility=filter


no, this is the result of this filter changes coupled with the order of 
ALL value filter or partial filters.


This also creates a mess with the storage of
table:database-range table:name=__Anonymous_Sheet_DB__0 
reducing the area within the range, depending when you save your file...



From 568c0635089964488a05043d7422eb40be15ccaf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pierre-Andr=C3=A9=20Jacquod?= pjacq...@alumni.ethz.ch
Date: Tue, 15 Nov 2011 20:55:53 +0100
Subject: [PATCH] fdo#42286 do not change selection area when determining filter values

Just use the current area, does not redetermine it in order to
find the list of values for drop-down of the filter.
---
 sc/source/core/data/documen3.cxx |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/sc/source/core/data/documen3.cxx b/sc/source/core/data/documen3.cxx
index 273af5f..14b0655 100644
--- a/sc/source/core/data/documen3.cxx
+++ b/sc/source/core/data/documen3.cxx
@@ -1395,7 +1395,6 @@ bool ScDocument::GetFilterEntries(
 ScDBData* pDBData = pDBCollection-GetDBAtCursor(nCol, nRow, nTab, false);  //!??
 if (pDBData)
 {
-pDBData-ExtendDataArea(this);
 SCTAB nAreaTab;
 SCCOL nStartCol;
 SCROW nStartRow;
-- 
1.7.3.4

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