Review Request 116949: close all tabs command added

[Jarosław Staniek]

> On March 21, 2014, 7:50 p.m., Jarosław Staniek wrote:
> > kexi/main/KexiMainWindow.cpp, line 210
> > <https://git.reviewboard.kde.org/r/116949/diff/1/?file=255904#file255904line210>
> >
> >     Looks complex...
> >     
> >     1.1. One hidden requirement is that closeWindowForTab() can return 'cancelled' value. In this case the routine should STOP closing the tabs immediately and return cancelled too (so return type shall be tristate, by the way). [You can observe the same behaviour when you press CANCEL for KDE Plasma or Windows desktops shutdown when application asks for saving changes]. 
> >     
> >     1.2. closeWindowForTab() can also return false what means something wrong happened and the tab stays open: we're keeping the tab open AND we continue closing other tabs.
> >     
> >     2. Given the above, wouldn't this be better:
> >     
> >     - collect a QList<KexiWindow*>
> >     - for each element of this list, execute KexiMainWindow::closeWindow(KexiWindow*); stop the loop if 'cancelled' was returned.
> >     
> >     This is safe no matter if any window refuses to close or not.
> >     
> >     Kexi does similar things on project closing.
> >     
> >     Please try to figure out if I am correct and also test the result.
> >     
> >     3. Moreover please study the coding style policies a bit: http://community.kde.org/Calligra/Developer_Info#Coding_style_and_licensing 
> >     
> >
> 
> Vishwa Modi wrote:
>     if closeWindowForTab() returns 'cancelled' value, are we supposed to stop closing all the remaining tabs?or we should continue closing others?
>

In this case the routine should STOP closing any remaining tabs, and return cancelled.


- Jarosław


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


On March 21, 2014, 1:37 p.m., Vishwa Modi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116949/
> -----------------------------------------------------------
> 
> (Updated March 21, 2014, 1:37 p.m.)
> 
> 
> Review request for Calligra and Jarosław Staniek.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> new KAction m_closeAction2 is added and closeAllTabs() function is added.
> 
> 
> Diffs
> -----
> 
>   kexi/main/KexiMainWindow.h 3d76bb2 
>   kexi/main/KexiMainWindow.cpp e916281 
> 
> Diff: https://git.reviewboard.kde.org/r/116949/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishwa Modi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140321/61b2f383/attachment.htm>