> On Jan. 26, 2016, 6:40 p.m., Aleix Pol Gonzalez wrote: > > Hi, > > I'm a bit afraid of all these ifndef. Do you think it would make sense to > > abstract out the KGlobalAccel usage? > > > > Otherwise, would it be possible to make KGlobalAccel useful (or just dumb) > > on Windows? > > Boudewijn Rempt wrote: > I would say: most applications do not need global accelerators, so making > kglobalaccel functional on windows is not really relevant, you wouldn't want > that dependency anyway because it doesn't add functionality. And building a > library and including it is always a burden, so I would say it's much better > to make it optional. > > Andre Heinecke wrote: > Hi, > > Abstracting it out would also be quite invasive imho. And from my > experience abstraced (optional) features are even harder to maintain then > ifdefs where you can easily see the codepath taken when something is not > available. While side effects through abstraction are harder to see when > hacking on code. > > As for the other point: > - GlobalAccel means that you basically need to have a keylogger on your > platform. I don't want that. There are system ways on Windows to create > global shortcuts already. Not as fine grained as KDE Actions but you could > use them to do command line calls. > - I don't really see this as a Windows only thing. KXmlGui provides very > useful features even without Global Shortcuts. And as for making KGlobalAccel > just dumb on Windows. While I think this would generally be a good idea I > expect that others (from Kde-Windows) who provide several KDE applications > and stuff like Systemsettings on Windows would disagree. > > We could add a dummy class in KXmlGui thats used if KGlobalAccel is not > available? This could avoid lots of ifdefs. But this would add a bit > maintenance cost when new API is used. While the IFDEFS make it quite > transparent to hackers that if you do thomething with GlobalAccel "please > guard it". > > Aleix Pol Gonzalez wrote: > @boud, yes, I also thought about your RR, in fact I looked it up but > couldn't find it. > > Ok, maybe it's actually the way to make xmlgui viable to deploy. > > Take this as a +1 by me. > > Martin Gräßlin wrote: > Could we maybe continue the route I went with making sure that > kglobalaccel doesn't start? I'm quite concerned about the ifdefs. If there > are still situations where kglobalacceld is started without being needed, > let's fix that. Let's make it a proper runtime thing instead of an ifdef > messery nobody will check. > > I'm quite concerned about making the change optional as that's a route to > breakage with distros and as the maintainer of kglobalaccel I'm not looking > forward to those bug reports. > > Boudewijn Rempt wrote: > Well, part of the point, for me at least, is not having the dependency at > all. Any extra library, especially one that adds no functionality but is just > present, is a burden just like #idefs are a burden. > > Martin Gräßlin wrote: > What can we do to make the burden not so hard on your side without adding > the ifdefs? KGlobalaccel is basically a tier 1 - the higher number is due to > the runtime part. Would it help to make the runtime part optional? Would it > help to have a BC drop in replacement which just no-ops? > > By doing the change as suggested here new burden is created and moved to > the shoulders of others. E.g. all Linux distributions which now have to be > more careful with packaging. So we need to find the right balance. > > Boudewijn Rempt wrote: > Well, for me personally it's water under the bridge. On the other hand, I > don't think that it's a burden for distributions: distributions always > install every dependency, even if it's optional. That is the big problem that > has led over the years to people complaining that Krita needed Marble, for > instance. > > Andre Heinecke wrote: > For me the build problem with KGlobalAccel is the build dependency to > DBus. BC drop in with No ops would help (in which case the configuration > entries should be completly hidden in the gui). But would a KGlobalAccel > without DBus / No-Ops be easier to maintain? > > And the best thing for me is that If I don't want some features to be > able not to build them at all instead of a replacement library. And a > KGlobalAccel "Dummy" as part of KXmlGui also appears wrong to me. > > Also my other two patches make DBus and KTextWidgets optional. For these > I definetly think that Ifdefs are the right way to go. > > > By doing the change as suggested here new burden is created and moved > to the shoulders of others. E.g. all Linux distributions which now have to be > more careful with packaging. So we need to find the right balance. > > I have to agree with Boudewijn there. We could of course only make it > optional on Windows but I would like to avoid making it a platform issue. > > Martin Gräßlin wrote: > > But would a KGlobalAccel without DBus / No-Ops be easier to maintain? > > if KGlobalAccel in it's current state is so bad that it needs to be > patched out of other frameworks, then yes KGlobalAccel needs to be modified. > Which is what I already did in the past, when it was brought to my attention > that just using xmlgui results in the runtime being started. > > Does it make sense to have a DBus free kglobalaccel? Certainly, on > non-Linux it doesn't make sense to use DBus. > > So the question to me is: is a stripped down KGlobalAccel (no DBus, no > runtime) sufficient to not get it patched out of other frameworks. If yes I > think that's the way to go. Is it work? Yes it is, but not that much. It's > only one source file with around 700 lines of code.
Well, what I am trying to say is that it's wrong to have a depedency on a library, a chunk of code, that doesn't add functionality to the application. - Boudewijn ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126895/#review91623 ----------------------------------------------------------- On Jan. 27, 2016, 8:53 a.m., Andre Heinecke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126895/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2016, 8:53 a.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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
