This switches the KCMs which previously used a bit of a hack to show the old Widgets based Get Hot New Stuff dialog from a button in the Qt Quick based KCMs so that they instead use the new KNewStuff Qt Quick submodule. The modules affected by this are:
| No Linters Available |
| No Unit Test Coverage |
| Buildable 21172 | |
| Build 21190: arc lint + arc unit |
i'm working on more of this (main reason for the status change, but thank you!), and there's a patch for KNS that wants to go in first (of course) - it's runtime, so it won't break the CI for this to go in first, but... still :)
Since D26543 is listed as a dependent, but it's in Frameworks, and Plasma 5.18 is going to depend on Frameworks 5.66, but 5.66 has already been tagged, that means that this can't land in 5.18, so we'll ship 5.18 with only the Global Themes KCM using the new dialog, and all the other dialogs using the old one. :/
Is D26543 a hard blocker, or just a nice-to-have bugfix?
Gah, i was under the impression the awkwardness with frameworks dependencies on the CI was due to Plasma requiring head...
Is D26543 a hard blocker, or just a nice-to-have bugfix?
Right, the functionality would all still exist (that is, install/uninstall/update/whatnot would function just fine), what would happen without the dependent patch is that the views will be out of sync with the system state (basically the onChangedEntriesChanged will just fail to call the function it's pointed at due to incompatible types, in all but the desktoptheme kcm (which just calls load anyway, which doesn't expect anything weird).
| kcms/icons/main.cpp | ||
|---|---|---|
| 175 | FWIW, there is an alternative approach that we do in plasma-workspace (shell/containmentconfigview.cpp) for reloading wallpapers types. kpackage emits a DBus signal when it performs an action, models can listen for this. We had to do that there because the model was quite separated from the UI. | |
| 275 ↗ | (On Diff #73597) | Does this code still exist? I can't see an equivalent in knewstuff. It's especially important on wayland |
In the case of three Frameworks (namely plasma-framework, kwayland and kwindowsystem) I have on file requests from the Plasma and KWin developers informing me that their software does indeed require latest HEAD (with no delay) of those particular frameworks, hence the convoluted setup on the CI system for those three. They do date back to May 2017 though.
All right, that's probably not a hard blocker, especially since this was already broken. So we can land this patch for 5.18 IMO.
This patch makes plasma-desktop fail to build without the dependent KNS framework change. That means that the KNS change is in fact a hard dependency and therefore this functionality can't make it into 5.18 with the patch's current state. If you want it for 5.18 (as I assume you do, and I do too!), you'll need to make D26543 not a dependency by only conditionally using the KNSCore::EntryWrapper functionality, or by finding a way to implement the fix in way that doesn't add new classes that have to be used here.
I notice that this also has the effect of changing the button texts to not have ellipses on the end. I guess NewStuff.Button needs to do that itself?
Works great BTW. A big improvement!
ah, d'oh, yes, you're quite right... it /is/ a hard dependency, i entirely forgot about that one... not entirely sure how i forgot about it, perhaps repression from having to write it at all, but yes. Right, let's see what we can do about that, then :)
There's a bug report for it (and that is indeed something that wants fixing in NewStuff.Button)
Works great BTW. A big improvement!
Thanks! :D
| kcms/icons/main.cpp | ||
|---|---|---|
| 175 | That does sound like something that would be good to look at at a later point in time (it'd make this whole thing vastly more involved, and i'm trying to keep it a bit more simple for now) | |
| 275 ↗ | (On Diff #73597) | The new dialog is entirely QtQuick (clicking the NewStuff.Button essentially just calls open() on a QtQuick.Dialogs Dialog, so if doing show on a QtQuick Dialog does the trick, then yes. Otherwise i am... severely uncertain of how that might be solved (though it would be central at least, since the code would be in KNSQuick). |
As discussed above, we need to be able to build against an older version of
Frameworks than 5.67. As such, wrap up the bits that aren't available before
then in some compile-time checks.
| kcms/colors/colors.cpp | ||
|---|---|---|
| 126–129 | This wraps a lot of existing functionality into that conditional as well. Was all of that stuff broken and useless without D26543? Everything seems to work anyway, but I'm curious to know if everything here needs to be wrapped up in this. | |
| kcms/cursortheme/kcmcursortheme.cpp | ||
| 413–421 | ditto | |
| kcms/colors/colors.cpp | ||
|---|---|---|
| 126–129 | All the code inside this conditional is for setting the most recent newly installed theme as the currently selected - perhaps a slightly long winded way of doing so, but yup, that's really all it does :) (and if it can't use the data being pulled out of the changedEntries, there doesn't seem any good reason to run the rest of the code which arguably would build and run, as it just wouldn't do anything) | |
| kcms/cursortheme/kcmcursortheme.cpp | ||
| 413–421 | Also ditto - all the code here depends on having an entry to work with, and if that can't be pulled out of changedEntries (which doesn't work until 5.67) then there's no particularly good reason to run the other bits of the code :) | |
Phab strikes again...
I would revert to a previous version of this patch using the history tab in the web UI, download the raw diff from the web UI, and then manually apply it on top of HEAD.
Let's try and unbreak this diff, shall we...
This change fails to build from source on FreeBSD due to missing headers in Frameworks.
Cancel that, it fails to build on Linux as well - i'm guessing that the KNetstuff changes have not been landed?
The problem is that a small part of it depends on functionality to exist master which doesn't currently exist - specifically D26543. I've pushed a thing that works around this, which can be reverted once said code has become available in the CI.
Now there are a lot of errors with localization.
There is a message about them.
https://bugs.kde.org/show_bug.cgi?id=415541
Temporarily fixed this problem with attached patches when building packages.
This error was originally described here https://phabricator.kde.org/D26665#594287
