D9300: Store screenMapping info only once
amantia closed this revision. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
amantia added a comment. I will merge this latest on Monday if there is no further objection REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
amantia added a comment. In https://phabricator.kde.org/D9300#179031, @broulik wrote: > Screen mapper is a singleton type, so it won't cause problems when one screen adds a mapping, it will propagate to the other containments automagically? Before we had `plasmoid.configuration` which signals changes but C++ does not, obviously. I'm not sure if that is a question for me or not. :) Due to being a singleton, I don't see a problem, all instances of the folderview see the same mapping. REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
broulik added a comment. Screen mapper is a singleton type, so it won't cause problems when one screen adds a mapping, it will propagate to the other containments automagically? Before we had `plasmoid.configuration` which signals changes but C++ does not, obviously. REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
hein added a comment. I'm totally on board with avoiding the redundant data for sure, just double-checking since it sets a precedent etc. REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
amantia added a comment. I think it is better to store once, than 2-3 times (per desktop) as the data is the same. I'm not sure if the corona level is the best, although I don't see big problems with it. REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
hein added a comment. Are you sure @mart? Do we really want to store applet-specific (though not applet instance-specific) config at the corona level? REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
mart accepted this revision. mart added a comment. fine with me, would like to still have Eike to chime in REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent, mart Cc: mart, mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D9300: Store screenMapping info only once
mlaurent accepted this revision. mlaurent added a comment. This revision is now accepted and ready to land. seems good for me REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik, mlaurent Cc: mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D9300: Store screenMapping info only once
amantia updated this revision to Diff 23846. amantia added a comment. Add const REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9300?vs=23821=23846 BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 AFFECTED FILES containments/desktop/package/contents/config/main.xml containments/desktop/package/contents/ui/FolderViewLayer.qml containments/desktop/plugins/folder/screenmapper.cpp To: amantia, #plasma, mwolff, hein, broulik Cc: mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D9300: Store screenMapping info only once
mlaurent added inline comments.
INLINE COMMENTS
> screenmapper.cpp:202
> +KConfigGroup group(config, QLatin1String("ScreenMapping"));
> +QStringList mapping =
> group.readEntry(QLatin1String("screenMapping"), QStringList{});
> +setScreenMapping(mapping);
const QStringList
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D9300
To: amantia, #plasma, mwolff, hein, broulik
Cc: mlaurent, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D9300: Store screenMapping info only once
amantia added reviewers: Plasma, mwolff, hein, broulik. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D9300 To: amantia, #plasma, mwolff, hein, broulik Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D9300: Store screenMapping info only once
amantia created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Until now every FolderView applet stored exactly the same screenMapping info in the config file. Store it only once using the config object of the Plasma::Corona REPOSITORY R119 Plasma Desktop BRANCH master REVISION DETAIL https://phabricator.kde.org/D9300 AFFECTED FILES containments/desktop/package/contents/config/main.xml containments/desktop/package/contents/ui/FolderViewLayer.qml containments/desktop/plugins/folder/screenmapper.cpp To: amantia Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
