----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115721/#review49870 -----------------------------------------------------------
Overloading KProcessRunner constructors with and without startupinfo doesn't seem necessary - this is all internal, no SC to keep. I would suggest to merge both into one (same with the overloaded run method), now that KStartupInfo is available everywhere. src/widgets/krun.cpp <https://git.reviewboard.kde.org/r/115721/#comment35039> duplicated from the #else. Could this be merged by doing #if HAVE_X11 if (isX11) { ... return ...; } #endif return KProcessRunner::run(...) ...; - David Faure On Feb. 13, 2014, 12:43 p.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115721/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2014, 12:43 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > ------- > > Reduce usage of HAVE_X11 in KRun and make it usable on non-X11 > > The diversion for some methods by having a KStartupInfoId is removed. > As KStartupInfoId is not an X11 specific class we can work with normal > method overloading instead of needing ifdefs. > > In addition the code which uses X11 specific functionality is adjusted > to be only used if we are on platform X11. For other platforms the > same fallback is used as already present through the ifdef diversion. > > > Diffs > ----- > > src/widgets/krun_p.h 05d63670c25a5c73fa6f690fdd73834167f9caf8 > src/widgets/krun.cpp 92e2fca4d122ef406566d1b8ebd04d37e7fe9746 > > Diff: https://git.reviewboard.kde.org/r/115721/diff/ > > > Testing > ------- > > used kruntest on X11 and Wayland - together with setting QT_QPA_PLATFORM env > variable we get the expected result. > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
