D26848: Don't assume the manager and menu have the same lifetime

2020-01-29 Thread David Redondo
This revision was automatically updated to reflect the committed changes. Closed by commit R265:a33675fdb3e4: Dont assume the manager and menu have the same lifetime (authored by davidre). REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE

D26848: Don't assume the manager and menu have the same lifetime

2020-01-29 Thread Kai Uwe Broulik
broulik accepted this revision. This revision is now accepted and ready to land. REPOSITORY R265 KConfigWidgets BRANCH menumanager (branched from master) REVISION DETAIL https://phabricator.kde.org/D26848 To: davidre, #frameworks, broulik Cc: anthonyfieroni, broulik,

D26848: Don't assume the manager and menu have the same lifetime

2020-01-24 Thread David Redondo
davidre marked 4 inline comments as done. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D26848 To: davidre, #frameworks Cc: anthonyfieroni, broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26848: Don't assume the manager and menu have the same lifetime

2020-01-23 Thread David Redondo
davidre updated this revision to Diff 74271. davidre added a comment. Use qApp as context REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26848?vs=74162=74271 BRANCH menumanager (branched from master) REVISION DETAIL

D26848: Don't assume the manager and menu have the same lifetime

2020-01-23 Thread David Redondo
davidre added inline comments. INLINE COMMENTS > kcolorschememanager.cpp:191 > QActionGroup *group = new QActionGroup(menu); > -connect(group, ::triggered, this, [this](QAction * action) > { > -activateScheme(d->model->index(action->data().toInt())); > +connect(group,

D26848: Don't assume the manager and menu have the same lifetime

2020-01-23 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > davidre wrote in kcolorschememanager.cpp:191 > What is meant by "connecting to qApp"? Just doing the scheme switching here? > I wanted to avoid the duplication that's why I created the static helper > method. Using `qApp` as context object

D26848: Don't assume the manager and menu have the same lifetime

2020-01-23 Thread David Redondo
davidre added inline comments. INLINE COMMENTS > kcolorschememanager.cpp:191 > QActionGroup *group = new QActionGroup(menu); > -connect(group, ::triggered, this, [this](QAction * action) > { > -activateScheme(d->model->index(action->data().toInt())); > +connect(group,

D26848: Don't assume the manager and menu have the same lifetime

2020-01-23 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > kcolorschememanager.cpp:191 > QActionGroup *group = new QActionGroup(menu); > -connect(group, ::triggered, this, [this](QAction * action) > { > -activateScheme(d->model->index(action->data().toInt())); > +

D26848: Don't assume the manager and menu have the same lifetime

2020-01-23 Thread Kai Uwe Broulik
broulik added a comment. lgtm REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D26848 To: davidre, #frameworks Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26848: Don't assume the manager and menu have the same lifetime

2020-01-22 Thread David Redondo
davidre added a reviewer: Frameworks. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D26848 To: davidre, #frameworks Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26848: Don't assume the manager and menu have the same lifetime

2020-01-22 Thread David Redondo
davidre created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. davidre requested review of this revision. REVISION SUMMARY The menu we retun can outlive the manager. So we can't use it or its members in slots when connecting to signals of