[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: kspread dependency formula error when a row or
From:       "Marijn Kruisselbrink" <m.kruisselbrink () student ! tue ! nl>
Date:       2010-05-06 18:14:51
Message-ID: 20100506181451.25490.1820 () localhost
[Download RAW message or body]



> On 2010-05-06 16:24:36, Marijn Kruisselbrink wrote:
> > trunk/koffice/kspread/Sheet.cpp, lines 952-966
> > <http://reviewboard.kde.org/r/3900/diff/1/?file=25743#file25743line952>
> > 
> > I don't think this change is entirely correct. In the old code the case where the \
> > referenced cell is a single cell that is in a row/column that was removed was \
> > handled correctly, and that case is no longer correct in the new code. Your new \
> > code only (presumably, I haven't tested it yet) handles it correctly for ranges, \
> > where the last cell in the range is in a row/column that is removed (it doesn't \
> > even seem to handle the case where it is the first cell of the range, in which \
> > case the adjustment should go the other way). Also since this is rather \
> > complicated to get right in all cases, it would be very nice to have some unit \
> > tests for all these cases.

Okay, I went ahead and added some unit tests for this code myself (see \
tests/TestSheet.cpp add in revision 1123729). Those tests document how I think \
reference should be modified when rows or columns are removed, and I think while this \
patch does fix some of the cases, it breaks other.


- Marijn


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3900/#review5462
-----------------------------------------------------------


On 2010-05-06 16:13:43, Pramod S G wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3900/
> -----------------------------------------------------------
> 
> (Updated 2010-05-06 16:13:43)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> The bug goes this way :
> Create a spreadsheet with : 
> A1 = 1
> A2 = 2
> A3 = 3
> A4 =  =sum(A1:A3)
> 
> Now, delete the third row. A dependency error something like =sum(A1:#Dependency!) \
> is noticed in A4(now, it is A3). 
> I've fixed this bug wherein A4 gets the correct summation. 
> 
> While fixing, I also realized that, the bug applies to the columns as well, which \
> I've fixed. Also, I noticed that, there is an incorrect value being calculated in \
> the resultant cell when you add a row/column previous to the resultant cell, which \
> I've corrected. 
> I've fixed all of the above.
> 
> 
> This addresses bug 136931.
> https://bugs.kde.org/show_bug.cgi?id=136931
> 
> 
> Diffs
> -----
> 
> trunk/koffice/kspread/Sheet.cpp 1123648 
> 
> Diff: http://reviewboard.kde.org/r/3900/diff
> 
> 
> Testing
> -------
> 
> Have successfully tested the patch against all of the above variants of bug.
> 
> 
> Thanks,
> 
> Pramod
> 
> 

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]