[WIP] [weather] Migrate nativeInterface config to plasmoid.configuration.*
Needs ReviewPublic

Authored by Zren on Feb 12 2019, 2:57 PM.

Details

Reviewers
kossebau
Summary

If plasmoid.nativeInterface.source != "", we copy it to plasmoid.configuration.source + same
with all other properties besides needsToBeSquare which is a "session" variable that updated
right after the migration code.

The selected weather services StringList is not a nativeInterface property, so it needs to be copied differently.


We could also copy the other properties the using the configValues().services pattern if you want the code consistent.

This is a pure QML solution. Ideally, the final patch will have a few more C++ functions/enums exposed to the QML. The DisplayUnits.qml properties could be turned into functions. Eg:

Utils.parseTemperatureUnitId(plasmoid.configuration.temperatureUnitId)

That way we don't have magic numbers (6001 // Celcius) littered throughout DisplayUnits.qml.


Ideally, we'll only keep the nativeInterface + migration code until Plasma 5.19 assuming Plasma 5.18 is the next Plasma LTS version. We could remove the migration code earlier, as having the user re-setup their weather widget isn't terribly cumbersome for the user.

Once it no longer depends on the nativeInterface, users can easily fork the default weather widget.

Immendiate benefits are that we no longer have to install the widget to .../share/plasma/plasmoids/ to test the widget's QML. We can now use plasmoidviewer -a package. Note that we still need to install to ~/.local/share/plasma/plasmoids/ to test the nativeInterface migration code as mentioned in the test plan.

Test Plan
  • Apply patch.
  • Copy weather/package to ~/.local/share/plasma/plasmoids/ and rename the package directory to org.kde.plasma.weather.
  • Restart plasmashell with kstart5 -- plasmashell --replace
  • The settings should have migrated, and the widget should still work.

Diff Detail

Repository
R114 Plasma Addons
Lint
Lint Skipped
Unit
Unit Tests Skipped
Zren created this revision.Feb 12 2019, 2:57 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 12 2019, 2:57 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
Zren requested review of this revision.Feb 12 2019, 2:57 PM

Thanks for the work on that patch!

So far I had thought of using the kconf_update system to ensure the config data would get the needed migration in the storage system (i.e. the ini files) into the [General] & Co. subsection as needed with the QML-based config variant.
Challenge here has been that so far nobody had pioneered to do that for Plasma config, at least for Plasma 5.

Doing this with some temporary logic in the applet itself of course is also a way. Though it breaks my technically-perfect-solution-aiming engineer's heart :)

This needs some more thinking from my side, so please be prepared for some 2 weeks time needed on my side to get over this :)

we no longer have to install the widget to .../share/plasma/plasmoids/

No experience here, but do we not still need to install the QML extension plugins to have the applet working? Or is there some mechanism to load them also from the build dir now?

Zren added a comment.Feb 12 2019, 6:34 PM

we no longer have to install the widget to .../share/plasma/plasmoids/

No experience here, but do we not still need to install the QML extension plugins to have the applet working? Or is there some mechanism to load them also from the build dir now?

  • We still need the private QML plugin for the Util.___ functions yes, but you don't need to reinstall those if you only touched the QML.
  • plasmoidviewer -a package does not populate plasmoid.nativeInterface, so any code touching the nativeInterface config (all of it) doesn't work when you use that to test. I'm not 100% sure why the relative path isn't loading the nativeInterface even though the metadata X-KDE-PluginInfo-Name should be the same. I'll need to dive into plasmoidviewer's code.
  • After the patch, we only need the nativeInterface to migrate the code. If it doesn't load the nativeInterface, plasma uses a placeholder QObject {}, which skips the migration code.

Though it breaks my technically-perfect-solution-aiming engineer's heart :)
please be prepared for some 2 weeks time needed on my side to get over this :)

lol, take your time. This is for the Plasma 5.16/5.17 release cycle so we've ages to choose this or develop a different patch.

So long as the migration code is in Ubuntu 20.04, and Debian 11 (Bullseye) in 2021. We can take it out before 22.04 or Debian 12 (Bookworm) in 2023. Anyone jumping 2 LTS versions usually expects some hiccups when upgrading.

Note: kicker used a similar migration method (QML onCompleted) to port their plasmoid.configuration.favorites => SQLiteDB.
https://github.com/KDE/plasma-desktop/blob/master/applets/kicker/package/contents/ui/main.qml#L115

Zren added a comment.Mar 5 2019, 4:29 PM

Was skimming the Plasma Scripting API today for something else and found where I saw the "update scripts" before:

It looks like you can only provide update scripts in the "shell" kpackage.
Leaving the links here for me to experiment with it in the future.

In D18961#410940, @Zren wrote:

Though it breaks my technically-perfect-solution-aiming engineer's heart :)
please be prepared for some 2 weeks time needed on my side to get over this :)

lol, take your time. This is for the Plasma 5.16/5.17 release cycle so we've ages to choose this or develop a different patch.

Those ages pass by quickly :/ Every other week on my mind shortly, but no action myself. And also killed your pace that way. Should either tell something upcoming WE or just give you green light.

kossebau resigned from this revision.Oct 21 2019, 11:11 PM

Still sorry for having stalled this effort. Stepping down as maintainer of the weather applet now, so also resigning here. Might be the chance to pick this up and just move on :)