D22363: Remove cursor theme option

[Filip Fila]

This revision was automatically updated to reflect the committed changes.
Closed by commit R123:34fc153b2593: Remove cursor theme option (authored by 
filipf).

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22363?vs=62174=62242

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

AFFECTED FILES
  sddmauthhelper.cpp
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  So much removed code.    Shipit!

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  remove-cursor-theme-option (branched from master)

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

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf updated this revision to Diff 62174.
filipf added a comment.


  address review comments

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22363?vs=62101=62174

BRANCH
  remove-cursor-theme-option (branched from master)

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

AFFECTED FILES
  sddmauthhelper.cpp
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> filipf wrote in advanceconfig.cpp:205
> I need the identifier to delete the entry in the config file (done in 
> sddmauthhelper.cpp), the value is not important so I just set whatever. Any 
> smarter way of doing this?

args[QStringLiteral("kde_settings.conf/Theme/CursorTheme")] = QVariant();

Then in sddmauthhelper, something like

  if (fileName == QLatin1String("kde_settings.conf")) {
  if (iterator.value().isValid()) {
  sddmConfig->group(groupName).writeEntry(keyName, 
iterator.value());
  } else {
 
sddmConfig->group(groupName).deleteEntry(keyName);
  
}
  sddmOldConfig->group(groupName).deleteEntry(keyName);
  }
  }

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf updated this revision to Diff 62101.
filipf added a comment.


  remove weird stuff from the .ui file

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22363?vs=62075=62101

BRANCH
  remove-cursor-theme-option (branched from master)

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

AFFECTED FILES
  sddmauthhelper.cpp
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf added inline comments.

INLINE COMMENTS

> davidedmundson wrote in sddmauthhelper.cpp:119
> Task for another day, we don't need to pass this in every time.
> 
> We can set this at compile time, using the same values from config.h as the 
> KCM would use

+1

> davidedmundson wrote in advanceconfig.cpp:205
> why?

I need the identifier to delete the entry in the config file (done in 
sddmauthhelper.cpp), the value is not important so I just set whatever. Any 
smarter way of doing this?

> davidedmundson wrote in advanceconfig.ui:220
> that seems odd?

Definitely does, QtDesigner can be weird. Going to undo that.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> sddmauthhelper.cpp:119
> +ActionReply reply = ActionReply::HelperErrorReply();
> +QSharedPointer sddmConfig = 
> openConfig(args[QStringLiteral("kde_settings.conf")].toString());
> +QSharedPointer sddmOldConfig = 
> openConfig(args[QStringLiteral("sddm.conf")].toString());

Task for another day, we don't need to pass this in every time.

We can set this at compile time, using the same values from config.h as the KCM 
would use

> advanceconfig.cpp:205
> +args[QStringLiteral("sddmUserConfig")] = sddmUserConfigPath;
> +args[QStringLiteral("kde_settings.conf/Theme/CursorTheme")] = 
> QStringLiteral("nothing");
>  

why?

> advanceconfig.ui:220
> +  
> +   ..
>   

that seems odd?

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf updated this revision to Diff 62075.
filipf added a comment.


  fix one things overlooked when manually fixing up the rebase

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22363?vs=62074=62075

BRANCH
  remove-cursor-theme-option (branched from master)

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

AFFECTED FILES
  sddmauthhelper.cpp
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf updated this revision to Diff 62074.
filipf added a comment.


  rebase on master

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22363?vs=61940=62074

BRANCH
  remove-cursor-theme-option (branched from master)

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

AFFECTED FILES
  sddmauthhelper.cpp
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

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


  This should now be completely functional. In order to proceed further, the 
dependent revision needs to be landed and then this patch rebased on master.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf updated this revision to Diff 61940.
filipf added a comment.


  remove CursorTheme entry in config file when resetting theme syncing

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22363?vs=61481=61940

BRANCH
  remove-cursor-theme-option (branched from master)

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

AFFECTED FILES
  kcm_sddm.actions
  sddmauthhelper.cpp
  sddmauthhelper.h
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22363: Remove cursor theme option

[Filip Fila]

filipf created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
filipf requested review of this revision.

REVISION SUMMARY
  Continuation of D22191  and is built on 
top of it. Instead of having a niche option for a cursor theme with all the 
underlying infrastructure having to support it, grab it from a KDE config file 
and write to the SDDM config file.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  remove-cursor-theme-option (branched from master)

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

AFFECTED FILES
  kcm_sddm.actions
  sddmauthhelper.cpp
  sddmauthhelper.h
  src/CMakeLists.txt
  src/advanceconfig.cpp
  src/advanceconfig.h
  src/cursortheme/cursortheme.cpp
  src/cursortheme/cursortheme.h
  src/cursortheme/dummytheme.cpp
  src/cursortheme/dummytheme.h
  src/cursortheme/sortproxymodel.cpp
  src/cursortheme/sortproxymodel.h
  src/cursortheme/thememodel.cpp
  src/cursortheme/thememodel.h
  src/cursortheme/xcursortheme.cpp
  src/cursortheme/xcursortheme.h
  src/ui/advanceconfig.ui

To: filipf
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart