D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread David Redondo
davidre closed this revision.

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

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread David Redondo
davidre added inline comments.

INLINE COMMENTS

> filipf wrote in sddmkcm.cpp:243
> Unfortunately not yet because we still need the `kdeglobals` file for color 
> schemes. From my previous testing it's not enough to just copy some line 
> `colorScheme=Adwaita`, we actually need to copy all of the color scheme 
> values written in `kdeglobals`. IIRC that was also a bit tricky just like 
> recreating the `fonts.conf` XML file, but it may be possible to reuse 
> existing KDE color scheme functions instead of copy-pasting from config files.

Alright

BRANCH
  arcpatch-D23257

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

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread Filip Fila
filipf added inline comments.

INLINE COMMENTS

> davidre wrote in sddmkcm.cpp:243
> Should I remove this then?

Unfortunately not yet because we still need the `kdeglobals` file for color 
schemes. From my previous testing it's not enough to just copy some line 
`colorScheme=Adwaita`, we actually need to copy all of the color scheme values 
written in `kdeglobals`. IIRC that was also a bit tricky just like recreating 
the `fonts.conf` XML file, but it may be possible to reuse existing KDE color 
scheme functions instead of copy-pasting from config files.

BRANCH
  arcpatch-D23257

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

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread David Redondo
davidre added inline comments.

INLINE COMMENTS

> sddmkcm.cpp:243
>  
>  if (!kdeglobalsPath.isEmpty()) {
>  args[QStringLiteral("kdeglobals")] = kdeglobalsPath;

Should I remove this then?

BRANCH
  arcpatch-D23257

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

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread Filip Fila
filipf added a comment.


  In D23257#677396 , @davidre wrote:
  
  > I rebased it because of the rewrite that happened, can you check if 
everything is still ok @filipf
  
  
  LGTM. Instead of copying the user's entire `kdeglobals` file (which could 
also be security risk), the sync function now only reads the font value from 
`kdeglobals` and writes it to the `kde_settings.conf` file. Thanks!

BRANCH
  arcpatch-D23257

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

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread David Redondo
davidre added a comment.


  I rebased it because of the rewrite that happened, can you check if 
everything is still ok @filipf

BRANCH
  arcpatch-D23257

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

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D23257: Allow for easier syncing of Plasma font

2021-02-18 Thread David Redondo
davidre updated this revision to Diff 83430.
davidre added a comment.


  Rebase

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23257?vs=64999=83430

BRANCH
  arcpatch-D23257

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

AFFECTED FILES
  src/sddmkcm.cpp

To: filipf, ngraham, #plasma, davidedmundson
Cc: davidre, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart