Re: Review Request 122590: Guard kglobalaccel against QApplication
On Bře. 5, 2015, 4:44 odp., Lukáš Tinkl wrote: For posterity, does this still work with khotkeys? Martin Gräßlin wrote: why should it not work with khotkeys? Or better said: what should be so special about khotkeys? Nothing in particular, I was just wondering - Lukáš --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review77063 --- On Úno. 26, 2015, 10:39 odp., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Úno. 26, 2015, 10:39 odp.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp d123af4 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated March 10, 2015, 9:21 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Changes --- Submitted with commit dcb5c0403c5caad901a46b2ae496078af0a90d12 by Kai Uwe Broulik to branch master. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp d123af4 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review77063 --- For posterity, does this still work with khotkeys? - Lukáš Tinkl On Úno. 26, 2015, 10:39 odp., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Úno. 26, 2015, 10:39 odp.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp d123af4 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
On March 5, 2015, 4:44 p.m., Lukáš Tinkl wrote: For posterity, does this still work with khotkeys? why should it not work with khotkeys? Or better said: what should be so special about khotkeys? - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review77063 --- On Feb. 26, 2015, 10:39 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 26, 2015, 10:39 p.m.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp d123af4 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review76860 --- Ship it! It's looking fine, assuming the auto tests still pass - Martin Gräßlin On Feb. 26, 2015, 10:39 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 26, 2015, 10:39 p.m.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp d123af4 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review76686 --- Ping, Frameworks people? - Kai Uwe Broulik On Feb. 16, 2015, 12:25 nachm., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 16, 2015, 12:25 nachm.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp df85547 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review76687 --- src/kglobalaccel.h https://git.reviewboard.kde.org/r/122590/#comment52819 from https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ You cannot: Remove a virtual function, even if it is a reimplementation of a virtual function from the base class -- you can make it do nothing though. - David Edmundson On Feb. 16, 2015, 12:25 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 16, 2015, 12:25 p.m.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp df85547 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 26, 2015, 9:39 nachm.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Changes --- Add back eventFilter method for BC but let it do nothing Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs (updated) - src/kglobalaccel.h 4a5595f src/kglobalaccel.cpp d123af4 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.cpp df85547 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review76117 --- src/kglobalaccel.cpp https://git.reviewboard.kde.org/r/122590/#comment52518 can you do setParent(this); instead of a manual delete? I know QWidget requires another QWidget in the ctor, but setParent from QObject should allow any. - David Edmundson On Feb. 16, 2015, 11:45 a.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 16, 2015, 11:45 a.m.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.cpp df85547 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/#review76116 --- we should investigate whether the argument for using a widget is still valid. The argument is that destroyed gives a QObject and not a QAction. but this can be easily worked around by connecting to a lambda and capturing the QAction. src/kglobalaccel.cpp https://git.reviewboard.kde.org/r/122590/#comment52517 it's totally fine to delete a nullptr. There's no need to guard it - Martin Gräßlin On Feb. 16, 2015, 12:45 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 16, 2015, 12:45 p.m.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs - src/kglobalaccel.cpp df85547 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122590: Guard kglobalaccel against QApplication
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122590/ --- (Updated Feb. 16, 2015, 11:56 vorm.) Review request for KDE Frameworks, David Edmundson and Martin Gräßlin. Changes --- Use QScopedPointer Repository: kglobalaccel Description --- KGlobalAccelPrivate uses a QWidget for some magic witchcraft which blows up in case there is no QApplication (kscreenlocker_greet). This guards against it. Diffs (updated) - src/kglobalaccel.cpp df85547 src/kglobalaccel_p.h b1528dc Diff: https://git.reviewboard.kde.org/r/122590/diff/ Testing --- screenlocker no longer blows up when adding the mpris dataengine which does kglobalaccel stuff but I have no idea whether this is the right approach and what the widget is actually for. Thanks, Kai Uwe Broulik ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
