----------------------------------------------------------- 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 > >
_______________________________________________ calligra-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/calligra-devel
