<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/116949/">https://git.reviewboard.kde.org/r/116949/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 21st, 2014, 7:50 p.m. UTC, <b>Jarosław Staniek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/116949/diff/1/?file=255904#file255904line210" style="color: black; font-weight: bold; text-decoration: underline;">kexi/main/KexiMainWindow.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KexiMainWindowTabWidget::paintEvent(QPaintEvent * event)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">210</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">KexiMainWindowTabWidget</span><span class="o">::</span><span class="n">closeAllTabs</span><span class="p">()</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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
</pre>
</blockquote>
<p>On March 21st, 2014, 8:46 p.m. UTC, <b>Vishwa Modi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">if closeWindowForTab() returns 'cancelled' value, are we supposed to stop closing all the remaining tabs?or we should continue closing others?
</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In this case the routine should STOP closing any remaining tabs, and return cancelled.</pre>
<br />
<p>- Jarosław</p>
<br />
<p>On March 21st, 2014, 1:37 p.m. UTC, Vishwa Modi wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra and Jarosław Staniek.</div>
<div>By Vishwa Modi.</div>
<p style="color: grey;"><i>Updated March 21, 2014, 1:37 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">new KAction m_closeAction2 is added and closeAllTabs() function is added.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kexi/main/KexiMainWindow.h <span style="color: grey">(3d76bb2)</span></li>
<li>kexi/main/KexiMainWindow.cpp <span style="color: grey">(e916281)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116949/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>