<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/126291/">https://git.reviewboard.kde.org/r/126291/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 25th, 2016, 8:26 a.m. CET, <b>Martin Gräßlin</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;">I don't like the introduction of the SCRAPBOOK. The repository is not the place for dead and old code. That's what git already supports with keeping the whole history.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We have some autotests for kwindowsystem. Could you try whether the tests work also on OSX?</p></pre>
</blockquote>
<p>On February 25th, 2016, 11:10 p.m. CET, <b>René J.V. Bertin</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;">I agree to a certain extent. Git's history feature isn't exactly comparable to a history book in which you can go look around to see if at some point maybe someone contributed some code that was never finished. At the very least you'd need to leave comments in the code that "here used to be some junk DNA that maybe could have led to a whole better world" (and in that case, why not just leave in the code #ifdeffed-out ...)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As to the autotests: they're built only when <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">X11_FOUND</code>. Are you in fact asking me to port them?</p></pre>
</blockquote>
<p>On February 26th, 2016, 7:48 a.m. CET, <b>Martin Gräßlin</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;">At the very least you'd need to leave comments in the code that "here used to be some junk DNA that maybe could have led to a whole better world" (and in that case, why not just leave in the code #ifdeffed-out ...)</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">eh no, we are not StarOffice. We have a version control system. No need to reference old code.</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;">they're built only when X11_FOUND. Are you in fact asking me to port them?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No, of course not. There are some which might not be X11 specific. E.g. kwindowsystem_threadtest.cpp. It would be good to know which ones can work on OSX</p></pre>
</blockquote>
<p>On February 26th, 2016, 10:25 a.m. CET, <b>René J.V. Bertin</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;">I'll see for the autotests.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My point is that you cannot expect that a future developer will study the old commit diffs to check if maybe they contained latent/experimental code that implemented an idea s/he was planning to test.
The experimental code I've moved is exactly of that nature; it aimed to implement a mechanism by which a table would be maintained of all windows belonging to all running applications. You pointed out correctly that it wouldn't compile and on my end I am not sure whether such a feature would actually be useful (never got feedback on the question I asked about that). So yeah, when the idea of cleaning out inactive code came up and you didn't seem adverse to the idea of keeping code snippets in another place for future reference I came up with the SCRAPBOOK idea.
Apparently I should have left in the code until I got the green light, and then remove it in an additional commit that does only that, with a sufficiently detailed commit message that the revision history could actually be of some real help here. I can't say I feel like moving all that code back to where it came from, so if you don't want the SCRAPBOOK file I guess it'll just do down the drain.</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;">Re: kwindowsystem_threadtest : that's the only one that doesn't require rewriting outside of the autotests CMake file. And it confirms a fear I had: there currently is no such thing as an internal list of owned windows, despite some efforts to maintain internal structures that allow to return something useful in certain situations (that I'll really need to look into again).
Maintaining such a list of windows feels a bit like a chicken-and-egg problem; can I rely on a Qt signal or do I need to use a lower-level API. The fact that most Qt/KDE applications do not have a Window menu like most Cocoa applications do (https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Tasks/UsingWindowsMenu.html) may be an indication that Qt does something peculiar.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Anyway, supposing I'm going to sit down and try to come up with a mechanism to maintain a list of the running app's open windows (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">WId</code>s). Can I presume that the list will be created (= plugin loaded) when there are no windows yet, meaning I only have to listen to window creation events? Or will I need to fetch the current list, add that, and then listen for window open events? I know how to go from a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">WId</code> to the underlying Cocoa instance, but the other way round may be quite a bit more tricky.</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On February 23rd, 2016, 10:54 p.m. CET, René J.V. Bertin 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 Software on Mac OS X and KDE Frameworks.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Feb. 23, 2016, 10:54 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwindowsystem
</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;">KWindowSystem has been lacking a platform plugin for OS X. This RR presents a "backport" of the modified KDE4 KWindowSystem implementation that has been used in the MacPorts kdelibs4 port for the last 2 or 3 (or more) years.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have made some initial steps to remove deprecated Carbon API calls, but this is clearly a work in progress.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Open questions include
- is there any justification to run an event handler (or Cocoa observer) to keep track of running applications, possibly even listing all their open windows?
- is there any use for the Qt event listener framework (cf. the NETEventFilter in the X11 plugin)? I haven't yet had time to try to figure out what this "could be good for", and am very open to suggestions in this departments.
- one application for such an event filter would be a listener that catches the opening and closing of all windows by the running process, and keeps track of their <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">WId</code>s. A new method could then be added (to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KWindowInfo</code>?) to distinguish <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">WId</code>s created by the running application from "foreign" ones (useful also on Wayland and MS Windows).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KWindowSystem::setMainWindow</code> should become a front for payload provided by the plugins. I'll leave that to the regular/official maintainer(s) of this framework.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This code could probably do with <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">lots</em> of comments; I'll try to add them as questions about this or that bit of code arise.</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;">On OS X 10.9.5 with Qt 5.5.1 and frameworks 5.16.0 .</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/debug_p.h <span style="color: grey">(5ef8996)</span></li>
<li>src/debug_p.cpp <span style="color: grey">(72abfb7)</span></li>
<li>src/kwindowsystem.cpp <span style="color: grey">(407a67d)</span></li>
<li>src/platforms/osx/CMakeLists.txt <span style="color: grey">(4fc3347)</span></li>
<li>src/platforms/osx/SCRAPBOOK <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/cocoa.json <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/kkeyserver.cpp <span style="color: grey">(3ddb921)</span></li>
<li>src/platforms/osx/kwindowinfo.cpp <span style="color: grey">(e8555bb)</span></li>
<li>src/platforms/osx/kwindowinfo.mm <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/kwindowinfo_mac_p.h <span style="color: grey">(c8f307e)</span></li>
<li>src/platforms/osx/kwindowinfo_p_cocoa.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/kwindowsystem.cpp <span style="color: grey">(1758829)</span></li>
<li>src/platforms/osx/kwindowsystem_mac_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/kwindowsystem_macobjc.mm <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/kwindowsystem_p_cocoa.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/plugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platforms/osx/plugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126291/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>