[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 126895: Make KGlobalAccel dependency in KXmlGui optional
From: Martin_Gräßlin <mgraesslin () kde ! org>
Date: 2016-01-27 7:02:37
Message-ID: 20160127070237.17778.24812 () mimi ! kde ! org
[Download RAW message or body]
--===============0006395257611885268==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126895/#review91635
-----------------------------------------------------------
Just in case that there is misunderstanding going on:
> Global Shortcuts (we don't have kded so this won't work for us anyway)
Global shortcut handling does not depend on kded!
Also I want to point out to \
http://commits.kde.org/kglobalaccel/40d047f013dc6f0e2f78066184a51a8c6e22eccd which \
was especially written for the behavior in xmlgui. As long as you don't configure a \
shortcut on a QAction it won't actually start the runtime part.
If the motivation is to ensure you don't need it on Krita on non-Linux: just don't \
ship the binary. You don't need it.
- Martin Gräßlin
On Jan. 26, 2016, 7:25 p.m., Andre Heinecke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126895/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2016, 7:25 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kxmlgui
>
>
> Description
> -------
>
> This is part of a three patch series that aims to allow a "leightweight" build of \
> KXmlGui without DBus and KService dependencies. I've added the patches to: \
> https://phabricator.kde.org/T1390 I'm not sure if I can create reviews that depend \
> on changes from another review, I'll try and if it does not work I'll open one \
> after another.
> Global shortcuts are a nice optional feature to have. But as they are not strictly \
> neccessary for the core functionality of KXmlGui, as I see it, and pull in an extra \
> dependency to DBus and need runtime support on the target platform they should be \
> optional.
> This (and the other changes) add lots of unloved ifdefs, I could understand if \
> thats disliked. But let me explain the background of this change:
> I'm currently updating Kleopatra in Gpg4win to a KDE Frameworks based build. This \
> is nice. Frameworks are awesome, I can just pick what I need and don't have \
> dependencies to lots of things that are actually not needed. Then comes KXmlGui, \
> adds 20 Framework dependencies, and I don't know what to do. I want:
> - configureable "KDE Style" GUI
> - configurable Shortcuts
> - KDE Standardactions (e.g. Help / WhatsThis)
> - kbugreport
> - KDE Integration in an KDE Environment
>
> But I don't want:
> - Global Shortcuts (we don't have kded so this won't work for us anyway)
> - DBus (our dbus is directory scoped and there are no other applications using dbus \
> installed by us)
> - KService dependency (System configuration has been troublesome in the past on \
> Windows and is not neccessary if we provide just a single installation)
> So these Patches are my way out of this Problem. Without the optional packages \
> KXmlGui provides what I want and does not depend on what I don't want.
>
> Diffs
> -----
>
> CMakeLists.txt 9d79619
> src/CMakeLists.txt 58f0c7a
> src/config-xmlgui.h.cmake 07c882f
> src/kactioncollection.cpp 9c45725
> src/kkeysequencewidget.cpp b2e2b6a
> src/kshortcuteditwidget.cpp 670d031
> src/kshortcutseditor.cpp 99dfb3d
> src/kshortcutseditoritem.cpp 461a90c
> src/kxmlguifactory.cpp 2767e69
>
> Diff: https://git.reviewboard.kde.org/r/126895/diff/
>
>
> Testing
> -------
>
> Compiled with and without dependency. Tested Kleopatra against it.
> Not yet tested on Windows, will do so in the next days.
>
>
> Thanks,
>
> Andre Heinecke
>
>
--===============0006395257611885268==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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/126895/">https://git.reviewboard.kde.org/r/126895/</a>
</td>
</tr>
</table>
<br />
<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;">Just \
in case that there is misunderstanding going on:</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;">Global Shortcuts (we don't have kded so this won't work for us anyway)</p> \
</blockquote> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Global shortcut handling does not depend on kded!</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Also I want to point out to \
http://commits.kde.org/kglobalaccel/40d047f013dc6f0e2f78066184a51a8c6e22eccd which \
was especially written for the behavior in xmlgui. As long as you don't configure a \
shortcut on a QAction it won't actually start the runtime part.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">If the motivation is to ensure you don't need it on Krita on non-Linux: \
just don't ship the binary. You don't need it.</p></pre> <br />
<p>- Martin Gräßlin</p>
<br />
<p>On January 26th, 2016, 7:25 p.m. CET, Andre Heinecke 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.</div>
<div>By Andre Heinecke.</div>
<p style="color: grey;"><i>Updated Jan. 26, 2016, 7:25 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kxmlgui
</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;">This is part of a three patch series that aims to \
allow a "leightweight" build of KXmlGui without DBus and KService dependencies. I've \
added the patches to: https://phabricator.kde.org/T1390 I'm not sure if I can create \
reviews that depend on changes from another review, I'll try and if it does not work \
I'll open one after another.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Global shortcuts are a nice optional \
feature to have. But as they are not strictly neccessary for the core functionality \
of KXmlGui, as I see it, and pull in an extra dependency to DBus and need runtime \
support on the target platform they should be optional.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This \
(and the other changes) add lots of unloved ifdefs, I could understand if thats \
disliked. But let me explain the background of this change:</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm \
currently updating Kleopatra in Gpg4win to a KDE Frameworks based build. This is \
nice. Frameworks are awesome, I can just pick what I need and don't have dependencies \
to lots of things that are actually not needed. Then comes KXmlGui, adds 20 Framework \
dependencies, and I don't know what to do. I want:
- configureable "KDE Style" GUI
- configurable Shortcuts
- KDE Standardactions (e.g. Help / WhatsThis)
- kbugreport
- KDE Integration in an KDE Environment</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">But I don't want:
- Global Shortcuts (we don't have kded so this won't work for us anyway)
- DBus (our dbus is directory scoped and there are no other applications using dbus \
installed by us)
- KService dependency (System configuration has been troublesome in the past on \
Windows and is not neccessary if we provide just a single installation)</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">So these Patches are my way out of this Problem. Without the optional \
packages KXmlGui provides what I want and does not depend on what I don't \
want.</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;">Compiled with and without dependency. Tested Kleopatra \
against it. Not yet tested on Windows, will do so in the next days.</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>CMakeLists.txt <span style="color: grey">(9d79619)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(58f0c7a)</span></li>
<li>src/config-xmlgui.h.cmake <span style="color: grey">(07c882f)</span></li>
<li>src/kactioncollection.cpp <span style="color: grey">(9c45725)</span></li>
<li>src/kkeysequencewidget.cpp <span style="color: grey">(b2e2b6a)</span></li>
<li>src/kshortcuteditwidget.cpp <span style="color: grey">(670d031)</span></li>
<li>src/kshortcutseditor.cpp <span style="color: grey">(99dfb3d)</span></li>
<li>src/kshortcutseditoritem.cpp <span style="color: grey">(461a90c)</span></li>
<li>src/kxmlguifactory.cpp <span style="color: grey">(2767e69)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126895/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============0006395257611885268==--
[Attachment #3 (text/plain)]
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic