[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]