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

[Camilla Boemann]

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


I have a hard time understanding how you can have tested this when it can't possobly build ?


libs/kotext/KoTextEditor.cpp (line 401)
<https://git.reviewboard.kde.org/r/124641/#comment57702>

    how can this compile when the variables are defined after ?


- Camilla Boemann


On Aug. 6, 2015, 10:14 a.m., Soma Schliszka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124641/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 10:14 a.m.)
> 
> 
> Review request for Calligra, Camilla Boemann and Thorsten Zachmann.
> 
> 
> 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
> -----
> 
>   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/2b2cadae/attachment.htm>