<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="http://git.reviewboard.kde.org/r/105563/">http://git.reviewboard.kde.org/r/105563/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2012, 6:25 a.m., <b>Oswald Buddenhagen</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="http://git.reviewboard.kde.org/r/105563/diff/11/?file=75565#file75565line239" style="color: black; font-weight: bold; text-decoration: underline;">kdm/backend/bootman.c</a>
<span style="font-weight: normal;">
(Diff revision 11)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">replaceInString(const char *str, const char *before, const char *after)</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">239</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="n">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p">,</span> <span class="n">ptr</span> <span class="o">=</span> <span class="n">buf</span><span class="p">;</span> <span class="o">*</span><span class="n">ptr</span><span class="p">;)</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;">this implementation is very clever, but i wonder whether it wouldn't be better to do something more conventional, like a loop of strstr() and memcpy().</pre>
</blockquote>
<p>On August 1st, 2012, 8:46 a.m., <b>Konstantinos Smanis</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;">Sure, I just made that up. If you want to go for clarity I can try a strstr implementation.</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;">yeah, it would be better. it will be probably not only easier to understand, but also shorter and faster.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2012, 6:25 a.m., <b>Oswald Buddenhagen</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="http://git.reviewboard.kde.org/r/105563/diff/11/?file=75565#file75565line281" style="color: black; font-weight: bold; text-decoration: underline;">kdm/backend/bootman.c</a>
<span style="font-weight: normal;">
(Diff revision 11)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">replaceInString(const char *str, const char *before, const char *after)</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">281</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <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;">if a closing brace can appear only alone, this line should become an else-if. if it can come after another statement, we need to think some more.</pre>
</blockquote>
<p>On August 1st, 2012, 8:46 a.m., <b>Konstantinos Smanis</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;">What do you mean here? A single if-else would require ANDing the external condition (line ending in '}').</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;">i don't quite understand what you mean, either.
anyway, even if the closing brace can follow other statements on the same line, its effect should not be visible until the other statements have been processed.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2012, 6:25 a.m., <b>Oswald Buddenhagen</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="http://git.reviewboard.kde.org/r/105563/diff/11/?file=75565#file75565line472" style="color: black; font-weight: bold; text-decoration: underline;">kdm/backend/bootman.c</a>
<span style="font-weight: normal;">
(Diff revision 11)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">getLilo(char ***opts, int *def, int *cur)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">getLilo(char ***opts, int *def, int *cur, const char **sep ATTR_UNUSED)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">368</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="p">(</span><span class="o">*</span><span class="n">get</span><span class="p">)(</span><span class="kt">char</span> <span class="o">***</span><span class="p">,</span> <span class="kt">int</span> <span class="o">*</span><span class="p">,</span> <span class="kt">int</span> <span class="o">*</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">452</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="p">(</span><span class="o">*</span><span class="n">get</span><span class="p">)(</span><span class="kt">char</span> <span class="o">***</span><span class="p">,</span> <span class="kt">int</span> <span class="o">*</span><span class="p">,</span> <span class="kt">int</span> <span class="o">*</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="kt"><span class="hl">char</span></span><span class="hl"> </span><span class="o"><span class="hl">**</span></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;">now you went kinda overboard - you are *both* using a complex normalized separator *and* announcing it. i think that given the former you can just remove the latter. i meant to imply that in my "ok, this all sucks, let's do it differently"-comment, but i clearly failed.</pre>
</blockquote>
<p>On August 1st, 2012, 8:46 a.m., <b>Konstantinos Smanis</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;">Should I hard-code it in ksmserver too then?</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;">yes</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2012, 6:25 a.m., <b>Oswald Buddenhagen</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="http://git.reviewboard.kde.org/r/105563/diff/11/?file=75571#file75571line485" style="color: black; font-weight: bold; text-decoration: underline;">ksmserver/shutdowndlg.cpp</a>
<span style="font-weight: normal;">
(Diff revision 11)
</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; ">KSMShutdownDlg::KSMShutdownDlg( QWidget* parent,</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">485</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">useSubmenus</span><span class="p">)</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;">why do you do that? the non-nested case should be just the simple solution of the general nested case.</pre>
</blockquote>
<p>On August 1st, 2012, 8:46 a.m., <b>Konstantinos Smanis</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;">I noticed that my menuentry-case code kinda duplicates the old code but could you please elaborate a little more on this? I am not sure I get how you want the implementation. If-nesting for submenus and simple actions outside of it?</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;">huh? i don't see why you need a special code path for "no submenus at all". it's just a normal submenu structure with all leaves hanging off the top-level menu.</pre>
<br />
<p>- Oswald</p>
<br />
<p>On August 1st, 2012, 8:54 a.m., Konstantinos Smanis wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Runtime and Oswald Buddenhagen.</div>
<div>By Konstantinos Smanis.</div>
<p style="color: grey;"><i>Updated Aug. 1, 2012, 8:54 a.m.</i></p>
<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;">Recent versions of GRUB2 introduce submenus which allow for nesting levels to appear (instead of the flat list in the past).
This patch consists of two parts: the parsing part in KDM (bootman.c) and creating a menu structure from the parsed list in ksmserver (shutdowndlg.*)
The parsing part produces a list like this:
Gentoo GNU/Linux
Advanced options for Gentoo GNU/Linux
Advanced options for Gentoo GNU/Linux>Gentoo GNU/Linux, with Linux 3.4.4
Advanced options for Gentoo GNU/Linux>Gentoo GNU/Linux, with Linux 3.4.4 (recovery mode)
Windows 7 (loader) (on /dev/sda2)
???? ?????????
which is then converted into the menu structure. These full identifiers can be properly used with `grub2-reboot`.
More info about submenus: http://ubuntuforums.org/showthread.php?p=10720316
Also check the related bug.
The parsing part of the patch can be applied in the KDE/4.9 and master branches as well (tested and working in KDE 4.8.95 and 4.9.x). ksmserver has migrated to QML since then however, and the menu structure has to wait. Currently it looks like this: http://i50.tinypic.com/96bw35.png
Related ML-discussion: http://lists.kde.org/?l=kde-core-devel&m=134160279511422&w=2
Update: There is a proper fix now for KDE/4.9 and master: https://git.reviewboard.kde.org/r/105568/</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Works with the menu file produced in my system with `grub2-mkconfig`.
Also works with a custom menu file I made (as shown in the second screenshot).</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=297209">297209</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdm/backend/bootman.c <span style="color: grey">(8b834d2)</span></li>
<li>kdm/backend/ctrl.c <span style="color: grey">(5d219fe)</span></li>
<li>kdm/backend/dm.h <span style="color: grey">(13e7b45)</span></li>
<li>kdm/backend/dm.c <span style="color: grey">(e0f1366)</span></li>
<li>kdm/backend/util.c <span style="color: grey">(6cd93ef)</span></li>
<li>ksmserver/shutdowndlg.h <span style="color: grey">(e5f0942)</span></li>
<li>ksmserver/shutdowndlg.cpp <span style="color: grey">(a09a1a7)</span></li>
<li>libs/kworkspace/kdisplaymanager.h <span style="color: grey">(76f25a7)</span></li>
<li>libs/kworkspace/kdisplaymanager.cpp <span style="color: grey">(28fabfc)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105563/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/105563/s/633/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/07/13/snapshot5_400x100.png" style="border: 1px black solid;" alt="Distribution's stock menu file" /></a>
<a href="http://git.reviewboard.kde.org/r/105563/s/634/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/07/13/snapshot6_400x100.png" style="border: 1px black solid;" alt="A custom menu file for testing" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>