D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Victor Ryzhykh]

victorr added a comment.


  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
  
  F7889061: knewstuff-5.66-tr.patch 
  F7889062: plasma-desktop-5.17.90-tr.patch 

  F7889063: plasma-workspace-5.17.90-fix.patch 

  F7889077: Before.jpeg 

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: victorr, meven, bcooksley, davidedmundson, ngraham, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D26544#595312 , @meven wrote:
  
  > In D26544#595269 , @leinir wrote:
  >
  > > In D26544#595246 , @bcooksley 
wrote:
  > >
  > > > Cancel that, it fails to build on Linux as well - i'm guessing that the 
KNetstuff changes have not been landed?
  > >
  > >
  > > The KNS changes have indeed not been landed yet
  >
  >
  > Please revert this temporally, it breaks all build of master plasma-desktop
  
  
  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: meven, bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Méven Car]

meven added a comment.


  In D26544#595269 , @leinir wrote:
  
  > In D26544#595246 , @bcooksley 
wrote:
  >
  > > Cancel that, it fails to build on Linux as well - i'm guessing that the 
KNetstuff changes have not been landed?
  >
  >
  > The KNS changes have indeed not been landed yet
  
  
  Please revert this temporally, it breaks all build of master plasma-desktop

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: meven, bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D26544#595246 , @bcooksley 
wrote:
  
  > Cancel that, it fails to build on Linux as well - i'm guessing that the 
KNetstuff changes have not been landed?
  
  
  The KNS changes have indeed not been landed yet

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Ben Cooksley]

bcooksley added a comment.


  Cancel that, it fails to build on Linux as well - i'm guessing that the 
KNetstuff changes have not been landed?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Ben Cooksley]

bcooksley added a comment.


  This change fails to build from source on FreeBSD due to missing headers in 
Frameworks.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:442c43b3ca98: Switch the Plasma Desktop KCMs to using 
KNewStuffQuick (authored by leinir).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26544?vs=73598=73638

REVISION DETAIL
  https://phabricator.kde.org/D26544

AFFECTED FILES
  kcms/colors/CMakeLists.txt
  kcms/colors/colors.cpp
  kcms/colors/colors.h
  kcms/colors/package/contents/ui/main.qml
  kcms/cursortheme/CMakeLists.txt
  kcms/cursortheme/kcmcursortheme.cpp
  kcms/cursortheme/kcmcursortheme.h
  kcms/cursortheme/package/contents/ui/main.qml
  kcms/desktoptheme/CMakeLists.txt
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/main.qml
  kcms/icons/CMakeLists.txt
  kcms/icons/main.cpp
  kcms/icons/main.h
  kcms/icons/package/contents/ui/main.qml
  kcms/ksplash/CMakeLists.txt
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h
  kcms/ksplash/package/contents/ui/main.qml

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir updated this revision to Diff 73598.
leinir added a comment.


  - Unbreak the merge

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26544?vs=73597=73598

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

AFFECTED FILES
  kcms/colors/CMakeLists.txt
  kcms/colors/colors.cpp
  kcms/colors/colors.h
  kcms/colors/package/contents/ui/main.qml
  kcms/cursortheme/CMakeLists.txt
  kcms/cursortheme/kcmcursortheme.cpp
  kcms/cursortheme/kcmcursortheme.h
  kcms/cursortheme/package/contents/ui/main.qml
  kcms/desktoptheme/CMakeLists.txt
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/main.qml
  kcms/icons/CMakeLists.txt
  kcms/icons/main.cpp
  kcms/icons/main.h
  kcms/icons/package/contents/ui/main.qml
  kcms/ksplash/CMakeLists.txt
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h
  kcms/ksplash/package/contents/ui/main.qml

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir updated this revision to Diff 73597.
leinir added a comment.


  - Unbreak the merge

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26544?vs=73595=73597

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

AFFECTED FILES
  CMakeLists.txt
  kcms/colors/CMakeLists.txt
  kcms/colors/colors.cpp
  kcms/colors/colors.h
  kcms/colors/package/contents/ui/main.qml
  kcms/cursortheme/CMakeLists.txt
  kcms/cursortheme/kcmcursortheme.cpp
  kcms/cursortheme/kcmcursortheme.h
  kcms/cursortheme/package/contents/ui/main.qml
  kcms/desktoptheme/CMakeLists.txt
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/main.qml
  kcms/icons/CMakeLists.txt
  kcms/icons/main.cpp
  kcms/icons/main.h
  kcms/icons/package/contents/ui/main.qml
  kcms/ksplash/CMakeLists.txt
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h
  kcms/ksplash/package/contents/ui/main.qml

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir updated this revision to Diff 73595.
leinir added a comment.


  Let's try and unbreak this diff, shall we...
  
  - Only try and work on newly installed themes
  - Port the cursortheme kcm to KNSQuick
  - Port the plasma theme kcm's ghns support to KNSQuick
  - Port the Icons kcm to KNSQuick
  - Port the Splash kcm to KNSQuick
  - Wrap the new requirements up in some handy checks, for older Frameworks

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26544?vs=73544=73595

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

AFFECTED FILES
  kcms/colors/CMakeLists.txt
  kcms/colors/colors.cpp
  kcms/colors/colors.h
  kcms/colors/package/contents/ui/main.qml
  kcms/cursortheme/CMakeLists.txt
  kcms/cursortheme/kcmcursortheme.cpp
  kcms/cursortheme/kcmcursortheme.h
  kcms/cursortheme/package/contents/ui/main.qml
  kcms/desktoptheme/CMakeLists.txt
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/main.qml
  kcms/icons/CMakeLists.txt
  kcms/icons/main.cpp
  kcms/icons/main.h
  kcms/icons/package/contents/ui/main.qml
  kcms/ksplash/CMakeLists.txt
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h
  kcms/ksplash/package/contents/ui/main.qml

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  Right, everything blew up i guess. Nice.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir updated this revision to Diff 73544.
leinir added a comment.


  - Merge remote-tracking branch 'origin/master' into switch-kcms-to-knsquick
  - Fix build (by removing stuff that was removed in master it seems)

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26544?vs=73409=73544

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

AFFECTED FILES
  CMakeLists.txt
  applets/icontasks/metadata.desktop
  applets/kicker/package/contents/ui/ActionMenu.qml
  applets/kicker/package/contents/ui/CompactRepresentation.qml
  applets/kicker/package/contents/ui/ConfigGeneral.qml
  applets/kicker/package/contents/ui/DashboardRepresentation.qml
  applets/kicker/package/contents/ui/DashboardTabBar.qml
  applets/kicker/package/contents/ui/DashboardTabButton.qml
  applets/kicker/package/contents/ui/ItemGridDelegate.qml
  applets/kicker/package/contents/ui/ItemGridView.qml
  applets/kicker/package/contents/ui/ItemListDelegate.qml
  applets/kicker/package/contents/ui/ItemListDialog.qml
  applets/kicker/package/contents/ui/ItemListView.qml
  applets/kicker/package/contents/ui/MenuRepresentation.qml
  applets/kicker/package/contents/ui/SideBarItem.qml
  applets/kicker/package/contents/ui/SideBarSection.qml
  applets/kicker/package/contents/ui/main.qml
  applets/kicker/package/metadata.desktop
  applets/kimpanel/backend/ibus/emojier/emojier.cpp
  applets/kimpanel/backend/ibus/emojier/org.kde.plasma.emojier.desktop
  applets/kimpanel/backend/ibus/emojier/resources.qrc
  applets/kimpanel/backend/ibus/emojier/ui/CategoryAction.qml
  applets/kimpanel/backend/ibus/emojier/ui/CategoryPage.qml
  applets/kimpanel/backend/ibus/emojier/ui/emojier.qml
  applets/kimpanel/package/contents/ui/main.qml
  applets/minimizeall/package/metadata.desktop
  applets/pager/package/contents/ui/main.qml
  applets/pager/plugin/windowmodel.cpp
  applets/showActivityManager/package/metadata.desktop
  applets/showdesktop/package/metadata.desktop
  applets/taskmanager/CMakeLists.txt
  applets/taskmanager/package/contents/ui/AudioStream.qml
  applets/taskmanager/package/contents/ui/MouseHandler.qml
  applets/taskmanager/package/contents/ui/PulseAudio.qml
  applets/taskmanager/package/contents/ui/Task.qml
  applets/taskmanager/package/contents/ui/ToolTipInstance.qml
  applets/taskmanager/package/contents/ui/code/layout.js
  applets/taskmanager/plugin/backend.cpp
  applets/taskmanager/plugin/backend.h
  applets/window-list/metadata.desktop
  containments/desktop/package/contents/ui/ConfigOverlay.qml
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/plugins/folder/autotests/positionertest.cpp
  containments/desktop/plugins/folder/foldermodel.cpp
  containments/panel/contents/ui/ConfigOverlay.qml
  design/plasmoids
  imports/activitymanager/CMakeLists.txt
  kcms/access/kcmaccess.desktop
  kcms/activities/BlacklistedApplicationsModel.cpp
  kcms/activities/CMakeLists.txt
  kcms/activities/ExtraActivitiesInterface.cpp
  kcms/activities/MainConfigurationWidget.cpp
  kcms/activities/MainConfigurationWidget.h
  kcms/activities/PrivacyTab.cpp
  kcms/activities/PrivacyTab.h
  kcms/activities/SwitchingTab.cpp
  kcms/activities/SwitchingTab.h
  kcms/activities/kactivitymanagerd_plugins_settings.kcfg
  kcms/activities/kactivitymanagerd_plugins_settings.kcfgc
  kcms/activities/kactivitymanagerd_settings.kcfg
  kcms/activities/kactivitymanagerd_settings.kcfgc
  kcms/activities/kcm_activities.desktop
  kcms/activities/qml/activitiesTab/ActivitiesView.qml
  kcms/activities/ui/PrivacyTabBase.ui
  kcms/activities/utils/dbusfuture_p.cpp
  kcms/activities/utils/dbusfuture_p.h
  kcms/autostart/autostart.cpp
  kcms/autostart/autostart.desktop
  kcms/baloo/CMakeLists.txt
  kcms/baloo/Messages.sh
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/kcm_baloofile.desktop
  kcms/baloo/package/metadata.desktop
  kcms/colors/CMakeLists.txt
  kcms/colors/colors.cpp
  kcms/colors/colors.h
  kcms/colors/colorsmodel.cpp
  kcms/colors/editor/org.kde.kcolorschemeeditor.desktop
  kcms/colors/filterproxymodel.cpp
  kcms/colors/kcm_colors.desktop
  kcms/colors/package/contents/ui/main.qml
  kcms/colors/package/metadata.desktop
  kcms/componentchooser/CMakeLists.txt
  kcms/componentchooser/EXAMPLE.desktop
  kcms/componentchooser/browser_settings.kcfg
  kcms/componentchooser/browser_settings.kcfgc
  kcms/componentchooser/componentchooser.cpp
  kcms/componentchooser/componentchooser.desktop
  kcms/componentchooser/componentchooser.h
  kcms/componentchooser/componentchooserbrowser.cpp
  kcms/componentchooser/componentchooserbrowser.h
  kcms/componentchooser/componentchooseremail.cpp
  kcms/componentchooser/componentchooseremail.h
  kcms/componentchooser/componentchooserfilemanager.cpp
  kcms/componentchooser/componentchooserfilemanager.h
  kcms/componentchooser/componentchooserterminal.cpp
  kcms/componentchooser/componentchooserterminal.h
  

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D26544#594194 , @ngraham wrote:
  
  > Thanks for the explanation!
  
  
  Not a problem, it's not necessarily easy to spot at a glance - and thanks! :D

REPOSITORY
  R119 Plasma Desktop

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Thanks for the explanation!

REPOSITORY
  R119 Plasma Desktop

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart, ngraham
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added inline comments.

INLINE COMMENTS

> ngraham wrote in colors.cpp:128
> 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.

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)

> ngraham wrote in kcmcursortheme.cpp:433
> ditto

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 :)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> colors.cpp:128
>  
> -const auto newEntries = m_newStuffDialog->installedEntries();
> -// If one new theme was installed, select the first color file 
> in it
> -if (newEntries.count() == 1) {
> -QStringList installedThemes;
> +#if KNEWSTUFFCORE_VERSION_MAJOR==5 && KNEWSTUFFCORE_VERSION_MINOR>=67
> +// If one new theme was installed, select the first color file in it

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.

> kcmcursortheme.cpp:433
>  {
> -KNS3::DownloadDialog dialog("xcursor.knsrc", nullptr);
> -if (dialog.exec()) {
> -KNS3::Entry::List list = dialog.changedEntries();
> -if (!list.isEmpty()) {
> -for (const KNS3::Entry& entry : list) {
> -if (entry.status() == KNS3::Entry::Deleted) {
> -for (const QString& deleted : entry.uninstalledFiles()) {
> -QVector list = 
> deleted.splitRef(QLatin1Char('/'));
> -if (list.last() == QLatin1Char('*')) {
> -list.takeLast();
> -}
> -QModelIndex idx = 
> m_themeModel->findIndex(list.last().toString());
> -if (idx.isValid()) {
> -m_themeModel->removeTheme(idx);
> -}
> +#if KNEWSTUFFCORE_VERSION_MAJOR==5 && KNEWSTUFFCORE_VERSION_MINOR>=67
> +for (int i = 0; i < changedEntries.count(); ++i) {

ditto

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir updated this revision to Diff 73409.
leinir added a comment.


  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.
  
  - Wrap the new requirements up in some handy checks, for older Frameworks

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26544?vs=73190=73409

BRANCH
  switch-kcms-to-knsquick (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D26544

AFFECTED FILES
  kcms/colors/CMakeLists.txt
  kcms/colors/colors.cpp
  kcms/colors/colors.h
  kcms/colors/package/contents/ui/main.qml
  kcms/cursortheme/CMakeLists.txt
  kcms/cursortheme/kcmcursortheme.cpp
  kcms/cursortheme/kcmcursortheme.h
  kcms/cursortheme/package/contents/ui/main.qml
  kcms/desktoptheme/CMakeLists.txt
  kcms/desktoptheme/kcm.cpp
  kcms/desktoptheme/kcm.h
  kcms/desktoptheme/package/contents/ui/main.qml
  kcms/icons/CMakeLists.txt
  kcms/icons/main.cpp
  kcms/icons/main.h
  kcms/icons/package/contents/ui/main.qml
  kcms/ksplash/CMakeLists.txt
  kcms/ksplash/kcm.cpp
  kcms/ksplash/kcm.h
  kcms/ksplash/package/contents/ui/main.qml

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added inline comments.

INLINE COMMENTS

> davidedmundson wrote in main.cpp:254
> 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. 
> But it has the advantage that we pick up external changes (i.e through 
> discover or whatever)

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)

> davidedmundson wrote in main.cpp:275
> Does this code still exist? I can't see an equivalent in knewstuff.
> 
> It's especially important on wayland

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).

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D26544#592286 , @ngraham wrote:
  
  > 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?
  
  
  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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir planned changes to this revision.
leinir added a comment.


  In D26544#592285 , @ngraham wrote:
  
  > 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.
  
  
  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 :)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D26544#591591 , @bcooksley 
wrote:
  
  > 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.
  
  
  Right, thanks for the clarification! :)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham added a comment.


  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!

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham added a comment.


  In D26544#591418 , @leinir wrote:
  
  > 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).
  
  
  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Ben Cooksley]

bcooksley added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: bcooksley, davidedmundson, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> main.cpp:254
>  
> -void IconModule::getNewStuff(QQuickItem *ctx)
> +void IconModule::ghnsEntriesChanged(const QQmlListReference )
>  {

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. 
But it has the advantage that we pick up external changes (i.e through discover 
or whatever)

> main.cpp:275
> -if (ctx && ctx->window()) {
> -m_newStuffDialog->windowHandle()->setTransientParent(ctx->window());
>  }

Does this code still exist? I can't see an equivalent in knewstuff.

It's especially important on wayland

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: davidedmundson, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D26544#591398 , @ngraham wrote:
  
  > 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. :/
  
  
  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).

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Nathaniel Graham]

ngraham added a comment.


  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?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26544: Switch the Plasma Desktop KCMs to using KNewStuffQuick

[Dan Leinir Turthra Jensen]

leinir requested review of this revision.
leinir added a comment.


  Right, that's the lot of the qtquick kcms in plasma-desktop!

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26544

To: leinir, mart
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart