D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Bhushan Shah]

bshah closed this revision.
bshah added a comment.


  This was merged in https://phabricator.kde.org/D21362 , thanks for your patch.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, #plasma, mart
Cc: bshah, rkflx, ngraham, mart, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  Ping? @progwolff? I just tried this out again in a fresh Neon dev unstable VM 
after rebasing on master and fixing the merge conflicts and got the same 
result: the settings in the KCM accurately depict the fontconfig's default 
settings, but they're not actually applied in the UI.
  
  Are you planning any further work on this?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  Ping? Is this ready for review again?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  Are there any other outstanding issues, or is this ready for review?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham resigned from this revision.
ngraham added a comment.


  OK, maybe there's something weird with my config somehow. :/
  
  Can anyone else reproduce? In the meantime @progwolff, maybe it would be best 
to continue anyway until and unless anyone else can confirm my issue. Is this 
reviewable now, or are you planning more changes?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  I installed a fresh Neon (dev unstable), removed the preinstalled files from 
/etc/fonts, copied your files to /etc/fonts and applied this patch.
  Still cannot reproduce your issue.
  
  F6216652: Screenshot_20180826_151000.png 


REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  I'm doing all my testing in a KDE Neon VM, FWIW. Would you be able to set 
that one up and try it there to see if you can reproduce it?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  I still cannot reproduce your issue. Any idea what I could try?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

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


  In D12925#302675 , @ngraham wrote:
  
  > With this latest version, the issue I mentioned earlier persists for me. I 
did briefly get sub-pixel rendering again to work by switching the hinting 
style to Full, but then after switching back to something else and then back to 
Full again, it no longer worked and I was back to no sub-pixel rendering again. 
Feels kinda buggy.
  
  
  Very strange...

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  With this latest version, the issue I mentioned earlier persists for me. I 
did briefly get sub-pixel rendering again to work by switching the hinting 
style to Full, but then after switching back to something else and then back to 
Full again, it no longer worked and I was back to no sub-pixel rendering again. 
Feels kinda buggy.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 38969.
progwolff added a comment.


  - fix writing config temporarily (previews) with missing local config entries

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=38829&id=38969

BRANCH
  arcpatch-D12925

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml
  kcms/fonts/previewimageprovider.cpp

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  Sure, here you go:
  F6167313: fonts.tar.gz 

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  could you give me all those files?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


cat ~/.config/fontconfig/fonts.conf 



 ~/.fonts
 
  
   true
  
 

  
  
  
ls /etc/fonts/conf.d
10-antialias.conf  57-dejavu-serif.conf
10-hinting-slight.conf 58-dejavu-lgc-sans-mono.conf
10-hinting.conf58-dejavu-lgc-sans.conf
10-scale-bitmap-fonts.conf 58-dejavu-lgc-serif.conf
11-lcdfilter-default.conf  60-latin.conf
20-unhint-small-dejavu-lgc-sans-mono.conf  64-language-selector-prefer.conf
20-unhint-small-dejavu-lgc-sans.conf   65-fonts-lmodern.conf
20-unhint-small-dejavu-lgc-serif.conf  65-fonts-persian.conf
20-unhint-small-dejavu-sans-mono.conf  65-fonts-takao-pgothic.conf
20-unhint-small-dejavu-sans.conf   65-khmer.conf
20-unhint-small-dejavu-serif.conf  65-nonlatin.conf
20-unhint-small-vera.conf  69-language-selector-zh-cn.conf
30-cjk-aliases.conf69-language-selector-zh-hk.conf
30-metric-aliases.conf 69-language-selector-zh-mo.conf
30-urw-aliases.conf69-language-selector-zh-sg.conf
40-nonlatin.conf   69-language-selector-zh-tw.conf
45-latin.conf  69-unifont.conf
49-sansserif.conf  70-no-bitmaps.conf
50-user.conf   80-delicious.conf
51-local.conf  90-synthetic.conf
56-neon-hack.conf  99-language-selector-zh.conf
56-neon-noto.conf  99pdftoopvp.conf
57-dejavu-sans-mono.conf   README
  
  I deliberately did not delete `~/.config/fontconfig/fonts.conf` and 
`~/.config/kcmfonts` as specified in the test plan because I wanted to test the 
upgrade case. :)

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  Works for me
  F6167260: Screenshot_20180731_164900.png 

  
  Could you show me your `~/.config/fontconfig/fonts.conf` and the content of 
`/etc/fonts/conf.d`?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  On the other hand, no matter what combination of sub-pixel antialiasing and 
hinting style I suggest, and regardless of what default value is shown in the 
UI, the result seems to be the functional equivalent of the None values. i.e. 
now sub-pixel anti-aliasing doesn't work at all.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  In D12925#301133 , @progwolff 
wrote:
  
  > I am not sure about the case when neither global nor local configs exist 
for subpixel or hint.
  >  On my system it seems like rgb subpixel rendering with slight hinting is 
used when no configs exist. But I cannot find any documentation on this.
  
  
  Yep me too. This makes sense since it's a sensible default. It looks like the 
old Vendor Default setting was actually introducing a bug and overriding the 
//actual// "vendor default"! As such, this patch fixes 
https://bugs.kde.org/show_bug.cgi?id=389598

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 38829.
progwolff added a comment.


  - fix previews

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=38828&id=38829

BRANCH
  arcpatch-D12925

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml
  kcms/fonts/previewimageprovider.cpp

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  I am not sure about the case when neither global nor local configs exist for 
subpixel or hint.
  On my system it seems like rgb subpixel rendering with slight hinting is used 
when no configs exist. But I cannot find any documentation on this.
  
  We also need to make sure that we don't reintroduce the bug fixed in 
https://phabricator.kde.org/R119:f02df03cb87b4bb5724eec668d49126a5f52a1e7
  I think we can safely remove the vendor default option that was introduced to 
fix this bug, if we make sure to never save unchanged configurations.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 38828.
progwolff added a comment.


  - Merge branch 'master' of git://anongit.kde.org/plasma-desktop into 
arcpatch-D12925
  - Merge branch 'master' of git://anongit.kde.org/plasma-desktop into 
arcpatch-D12925
  - also drop vendor-default-option for subpixel and hint

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=38764&id=38828

BRANCH
  arcpatch-D12925

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml
  kcms/fonts/previewimageprovider.cpp

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a reviewer: Plasma.
ngraham added a comment.
This revision now requires changes to proceed.


  Nice! Two nitpicks:
  
  - `Anti-aliasing: [x] Use antialiasing` Either change the checkbox's label to 
"Enable", or remove the left label and give this whole section its own header.
  - We've still got a "Vendor Default" option for "Sub-pixel rendering type". 
Instead, there should really be a "none" option there to complement the options 
for the different types.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, ngraham, #plasma
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 38764.
progwolff added a comment.


  - split changes to previewimageprovider.cpp to a separate commit

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=38757&id=38764

BRANCH
  arcpatch-D12925

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  Big sorry for being away for so long. I had some things to do that took much 
more time than initally planned.
  Many thanks to those who fixed the bugs I introduced or created workarounds...
  
  I tried to get this patch working now. First of all, I want to say a few 
words about how this patch will remove the "vendor default" option.
  Previously "vendor default" ment that no anti-aliasing entry is written to 
the local config file. There could however be a global config file that has 
such an entry.
  As it was already said in some older comments, this is nothing special and 
many settings exist in other kcms or other applications where we can have both 
local and global config files. So, we could drop the "vendor default" option 
here, if we are able to parse the global configs.
  
  This patch tries to do this by using all config files given by 
`FcConfigGetConfigFiles`. 
  Ater parsing all the config files we know if there is a local 
anti-aliasing-entry. If there is, any changes will be written to this entry in 
the local config file. 
  If there is not a local anti-aliasing-entry, none will be created unless the 
option is actually changed in the kcm. If it is changed, a new entry is created 
in the local config file.
  If the user resets the defaults, the local entry is removed.
  
  I tested this on my machine without any problems. We should however test this 
carefully on different systems. There might be some cases I did not think about 
yet.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 38757.
progwolff added a comment.


  Rebase on master. Partial rewrite.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=34315&id=38757

BRANCH
  arcpatch-D12925

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml
  kcms/fonts/previewimageprovider.cpp

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  Any update on this patch?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  In D12925#264085 , @rkflx wrote:
  
  > I'll resume testing once this works a bit better. I'd say let's not rush 
this, and get it right for 5.14.
  
  
  I agree. Thanks for investigating!

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Henrik Fehlauer]

rkflx added a comment.


  Hm, repeatedly pressing the Defaults button works really weird for me, and in 
some cases clicking on Disabled from… does not activate the option and only 
enables the widgets below (I need a second click for actually switching the 
radio selection).
  
  I'll resume testing once this works a bit better. I'd say let's not rush 
this, and get it right for 5.14.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Henrik Fehlauer]

rkflx added a comment.


  You might want to rebase on the latest changes to D12849 
 (currently the patch only applies against 
`arc pat --diff 34209`, i.e. a previous version of D12849 
), and here set D12849 
 as a dependent Diff. See also 
https://community.kde.org/Infrastructure/Phabricator#Marking_patches_as_dependent_on_other_patches.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff
Cc: rkflx, ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Henrik Fehlauer]

rkflx resigned from this revision.
rkflx added a comment.


  Sorry for resigning as a reviewer, but currently I lack the time to review 
this in depth. Maybe I can do some basic testing tomorrow, but no promises.
  
  As said before though, conceptually this makes sense to me. IIUC, this is a 
GUI for the fontconfig setting, and no GUI to bypass the fontconfig setting.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff
Cc: ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 34315.
progwolff added a comment.


  - remove unintentionally committed debug lines

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=34274&id=34315

BRANCH
  fonts_kcm_globalConfigFiles (branched from master)

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml

To: progwolff, rkflx
Cc: ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Nathaniel Graham]

ngraham added a comment.


  +1 conceptually; "Vendor Default" was always really awkward to explain to 
people.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, rkflx
Cc: ngraham, mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff added a comment.


  In D12925#263459 , @mart wrote:
  
  > if it removes the vendor default option, how can i ask to not touch the 
global/local defaults in any way?
  
  
  Just don't change anything in the kcm.
  The global config files are never touched and the kcm controls are set to the 
global config by default.

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, rkflx
Cc: mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Marco Martin]

mart added a comment.


  if it removes the vendor default option, how can i ask to not touch the 
global/local defaults in any way?

REPOSITORY
  R119 Plasma Desktop

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

To: progwolff, rkflx
Cc: mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff updated this revision to Diff 34274.
progwolff added a comment.


  - Fix 'disabled from'

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12925?vs=34272&id=34274

BRANCH
  fonts_kcm_globalConfigFiles (branched from master)

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml

To: progwolff, rkflx
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12925: Parse global config files. Remove 'Vendor default' option. Fix changes not recognized.

[Julian Wolff]

progwolff created this revision.
progwolff added a reviewer: rkflx.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
progwolff requested review of this revision.

REVISION SUMMARY
  The fonts kcm did only parse a local config file. Global and default settings 
were not recognized.
  With this patch, all global config files are parsed before the local file is 
parsed.
  This allows us to drop the "Vendor default" option.
  
  This patch also fixes some changes not recognized (apply button disabled).

TEST PLAN
  Delete ~/.config/fontconfig/fonts.conf.
  Start `kcmshell5 fonts`. The default/global settings should be displayed. 
  Change some settings. The apply button should be enabled.
  Save. A new config file should be created.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fonts_kcm_globalConfigFiles (branched from master)

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

AFFECTED FILES
  kcms/fonts/fonts.cpp
  kcms/fonts/fonts.h
  kcms/fonts/kxftconfig.cpp
  kcms/fonts/kxftconfig.h
  kcms/fonts/package/contents/ui/main.qml

To: progwolff, rkflx
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart