Review Request 124641: Refactoring KoTextEditor::recursivelyVisitSelection() to make it cleaner and easier to understand

Soma Schliszka soma.schliszka at gmail.com
Thu Aug 6 12:48:12 BST 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124641/
-----------------------------------------------------------

(Updated Aug. 6, 2015, 11:48 a.m.)


Review request for Calligra, Camilla Boemann and Thorsten Zachmann.


Changes
-------

Uploaded the latest diff where the variables are at their right place.


Repository: calligra


Description
-------

The KoTextEditor::recursivelyVisitSelection() function had a bit complex conditional expression when there is a table inside the selection. 
This change is following the previous behaviour but the expression is built more logically. It isn't the shortest version, but it's much easier to understand and simpler to use.
There are several cases of selecting a table, and it's not trivial to say whether entire or just a part is selected.

* The process has been divided into two section: first, the conditional expression decides about the visiting mode (party or entirely), than just 'visit' the objects in order.
* I wasn't sure about why is it necessary to select the entire table in a different way? If the entire table is selected, than selectedTableCells() should do the same. That's why there is no different caret-selection-handling right before the visit.
* Other modification is the removed duplicate of cell-protection-check. If only one cell is selected, the loop still can operate, just runs only once.
* The patch contains many inline comments to cover all cases clearly.

*This patch is just a suggestion to make this snippet more cleaner.*


Diffs (updated)
-----

  libs/kotext/KoTextEditor.cpp 392f682 

Diff: https://git.reviewboard.kde.org/r/124641/diff/


Testing
-------

Builded successfully, table works the same as before.


Thanks,

Soma Schliszka

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150806/534cfb64/attachment.htm>


More information about the calligra-devel mailing list