D10388: allow to have more than one default activity
This revision was automatically updated to reflect the committed changes. Closed by commit R161:6e60c6a3e46b: allow to have more than one default activity (authored by mart). REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10388?vs=27224&id=27327 REVISION DETAIL https://phabricator.kde.org/D10388 AFFECTED FILES src/service/Activities.cpp To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
ivan accepted this revision. This revision is now accepted and ready to land. REPOSITORY R161 KActivity Manager Service BRANCH phab/defaultMultipleActivities REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
mart marked 2 inline comments as done. mart added a comment. sorry if it taken so long to answer, issues should be adressed now :) REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
mart updated this revision to Diff 27224. mart added a comment. - fix syntax REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10388?vs=26768&id=27224 BRANCH phab/defaultMultipleActivities REVISION DETAIL https://phabricator.kde.org/D10388 AFFECTED FILES src/service/Activities.cpp To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
davidedmundson added inline comments. INLINE COMMENTS > mart wrote in Activities.cpp:233 > setActivityState(activity, Running); > makes it running but *not* current, right? yeah, my bad. Ignore my comment. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
mart added inline comments. INLINE COMMENTS > ivan wrote in Activities.cpp:233 > @davidedmundson Haven't looked at that part of Plasma for some time, but I > see no reason why it would do that since creating a new activity does not > make it current. setActivityState(activity, Running); makes it running but *not* current, right? REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
ivan added inline comments. INLINE COMMENTS > mart wrote in Activities.cpp:233 > when i tested it it didn't seem to switch to the latest created one without > an explicit setCurrentActivity? @davidedmundson Haven't looked at that part of Plasma for some time, but I see no reason why it would do that since creating a new activity does not make it current. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
mart added inline comments. INLINE COMMENTS > davidedmundson wrote in Activities.cpp:233 > If the default is "activityA, activityB" I don't want clients (plasmashell) > to start loading the layout for activityA and then start loading activityB. when i tested it it didn't seem to switch to the latest created one without an explicit setCurrentActivity? REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
davidedmundson added inline comments. INLINE COMMENTS > ivan wrote in Activities.cpp:233 > What do you mean by 'toggling' here? If the default is "activityA, activityB" I don't want clients (plasmashell) to start loading the layout for activityA and then start loading activityB. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
ivan added inline comments.
INLINE COMMENTS
> Activities.cpp:125
> +//NOTE: config key still singular for retrocompatibility
> +const QStringList names = cg.readEntry("defaultActivityName",
> QStringList({i18n("Default")}));
> +
`QStringList{i18n("Default")}` should be enough
> Activities.cpp:127
> +
> +for (const QString &name : names) {
> +QMetaObject::invokeMethod(
`const auto&` - IMO, no need to specify the type explicitly here
> davidedmundson wrote in Activities.cpp:233
> We'll end up toggling between activities on startup.
> Can we avoid this?
What do you mean by 'toggling' here?
REPOSITORY
R161 KActivity Manager Service
REVISION DETAIL
https://phabricator.kde.org/D10388
To: mart, #plasma, ivan, bshah
Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
davidedmundson added inline comments. INLINE COMMENTS > Activities.cpp:233 > > setActivityState(activity, Running); > We'll end up toggling between activities on startup. Can we avoid this? REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
mart added reviewers: ivan, bshah. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D10388 To: mart, #plasma, ivan, bshah Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D10388: allow to have more than one default activity
mart created this revision. mart added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. mart requested review of this revision. REVISION SUMMARY since addActivity in layout.js is the wrong place and extremely fragile, allow to have more than one default activity with custom names the config key is unchanged so all is retrocompatible, even if the key being singular isn't 100% correct now TEST PLAN tried with an empty session, works correctly REPOSITORY R161 KActivity Manager Service BRANCH phab/defaultMultipleActivities REVISION DETAIL https://phabricator.kde.org/D10388 AFFECTED FILES src/service/Activities.cpp To: mart, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
