[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice
Subject: Re: patch for kspread
From: David Faure <david () mandrakesoft ! com>
Date: 2000-09-02 17:29:04
[Download RAW message or body]
>So could you test it please.
Can't test, but upon reading:
- if ( selection.left() == 0 || (selection.width()==1 &&
selection.height()==1))
+ KSpreadCell *cell=activeTable()->cellAt(markerColumn(),markerRow());
+ QRect extraCell;
+ extraCell.setCoords(markerColumn(),markerRow(),
+ markerColumn()+cell->extraXCells(),markerRow()+cell->extraYCells());
+ if ( selection.left() == 0 || (selection.width()==1 &&
selection.height()==1)
+ ||extraCell==selection)
Can't you simplify the test so that it's only
if ( selection.left() == 0 || extraCell == selection )
?
After all, a cell with no extra cells is just a particular case,
so the test "if the selection is only one cell" will still return true
because of extraCell==selection.
This doesn't change anything compared to the suggested patch, but
makes maintainance easier by simplifying the code :-)
About the paintEvents, it looks like
selection.left() != 0 && (selection.width()!=1 || selection.height()!=1) &&
extraCell!=selection
could be put in a bool out of the loop, to avoid testing this at each
iteration of the loop (and save a bit of time).
Only what depends on x (or y) should be processed inside the loop.
PS: some comments in the code would help. Not too much, just a bit :-)
The above code could be commented "// No selection, or only one cell selected".
PS2: use pruneemptydirs or remove by hand all the '?' stuff at the beginning
of your diffs (pics/@kde_minidir@ etc.) :-)
David.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic