Review Request 116949: close all tabs command added

[Jarosław Staniek]

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



kexi/main/KexiMainWindow.h
<https://git.reviewboard.kde.org/r/116949/#comment37651>

    coding standards: let's use better name



kexi/main/KexiMainWindow.cpp
<https://git.reviewboard.kde.org/r/116949/#comment37653>

    &C accelerator is used already in this context menu (line 170) so let's use something other, e.g. &o



kexi/main/KexiMainWindow.cpp
<https://git.reviewboard.kde.org/r/116949/#comment37654>

    coding std: missing spaces



kexi/main/KexiMainWindow.cpp
<https://git.reviewboard.kde.org/r/116949/#comment37655>

    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 
    
    


- Jarosław Staniek


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/054bbdb7/attachment.htm>