<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/129892/">https://git.reviewboard.kde.org/r/129892/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Януари 27th, 2017, 7:01 преди обяд EET, <b>Anthony Fieroni</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So, wait a minute i just quote you
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">This patch attempts to fix it by iterating all the main windows when they are shown and if all the menubars of all mainwindows are native, then hides the show menu bar action (basically erasing it from existence).</em> from https://git.reviewboard.kde.org/r/129261/
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">This has nothing to do with whether you are using Unity or Plasma</em> from https://git.reviewboard.kde.org/r/129885/
I want an explanation, i never use Unity, i will never use it, but i want know in Unity - Kate, Dolphin etc. on showEvent allMenuBarsNative == true, wasNative == true ?
If so, i'm -1 for this approach</p></pre>
</blockquote>
<p>On Януари 28th, 2017, 4:27 след обяд EET, <b>Albert Astals Cid</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You don't seem to understand the code, what makes you think you're able to give a -1 when you don't understand the code?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You want an explanation? What makes you think you should be given one? You're not the maintainer of the code, you have not contributed at all to kconfigwidgets and on top of that you've been nothing other than agressive both in this and in the other review request.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Still i'll try to explain it to you, but don't think your attitude is helping, it's not.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">but i want know in Unity - Kate, Dolphin etc. on showEvent allMenuBarsNative == true, wasNative == true ?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This depends if it's the first show or not. On the first show obviously wasNative will be false since it is how it is initialized in the constructor. So on the first show it'll be allMenuBarsNative == true, wasNative == false and on the second show, yes it will be allMenuBarsNative == true, wasNative == true.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I sincerely think the code is quite easy to understand, what you don't understand?</p></pre>
</blockquote>
<p>On Януари 28th, 2017, 5:03 след обяд EET, <b>Anthony Fieroni</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So when i know allMenuBarsNative == true, wasNative == true, why you think that i'm not uderstainding? I'm against to store / restore menubar chacked and visabiliity.</p></pre>
</blockquote>
<p>On Януари 28th, 2017, 5:29 след обяд EET, <b>Anthony Fieroni</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This depends if it's the first show or not. On the first show obviously wasNative will be false since it is how it is initialized in the constructor. So on the first show it'll be allMenuBarsNative == true, wasNative == false and on the second show, yes it will be allMenuBarsNative == true, wasNative == true.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Albert, i know that, that's why i made that patch https://git.reviewboard.kde.org/r/129885/</p></pre>
</blockquote>
<p>On Януари 28th, 2017, 6:58 след обяд EET, <b>Albert Astals Cid</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do you know what a native menubar is? Can you explain why you are against storing the state?</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I read this http://doc.qt.io/qt-5/qmenubar.html#nativeMenuBar-prop, personally i never use nor OSX nor Unity. I'm against because this is a bad practive, yeah i see new patch will work (logically, because i'm not test it) but i don't see any reason to do this when we can do it without. Now we insert new code rather than minimize it, my approach when i write code are:
1. simple logic
2. less code
3. less temporary state changes
4. less variables
For me new code doesn't meet these conditions, i'm not a person who decide who will be pushed or not.</p></pre>
<br />
<p>- Anthony</p>
<br />
<p>On Януари 27th, 2017, 12:19 преди обяд EET, Albert Astals Cid wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Anthony Fieroni and David Faure.</div>
<div>By Albert Astals Cid.</div>
<p style="color: grey;"><i>Updated Ян. 27, 2017, 12:19 преди обяд</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kconfigwidgets
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It makes no sense to treat no menubars as all menubars native so just bail early, the app is probably still in construction if there's no menubars but it actually has a showmenubaraction.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Dolphin behaves better regarding the show menubar action.</p></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>src/kstandardaction.cpp <span style="color: grey">(6e5bba7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129892/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>