Store screenMapping info only once
ClosedPublic

Authored by amantia on Dec 12 2017, 6:21 PM.

Details

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

Diff Detail

Repository
R119 Plasma Desktop
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
amantia created this revision.Dec 12 2017, 6:21 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 12 2017, 6:21 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
amantia requested review of this revision.Dec 12 2017, 6:21 PM
mlaurent added inline comments.
containments/desktop/plugins/folder/screenmapper.cpp
202

const QStringList

amantia updated this revision to Diff 23846.Dec 13 2017, 8:37 AM

Add const

mlaurent accepted this revision.Dec 13 2017, 9:05 AM

seems good for me

This revision is now accepted and ready to land.Dec 13 2017, 9:05 AM
mart accepted this revision.Dec 13 2017, 9:40 AM
mart added a subscriber: mart.

fine with me, would like to still have Eike to chime in

hein added a comment.Dec 13 2017, 9:59 AM

Are you sure @mart? Do we really want to store applet-specific (though not applet instance-specific) config at the corona level?

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.

hein added a comment.Dec 13 2017, 10:05 AM

I'm totally on board with avoiding the redundant data for sure, just double-checking since it sets a precedent etc.

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.

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.

I will merge this latest on Monday if there is no further objection

amantia closed this revision.Dec 18 2017, 10:23 AM