[GTK Config] Parameterise GTK config editing
AcceptedPublic

Authored by gikari on Feb 2 2020, 1:45 PM.

Details

Reviewers
cblack
Group Reviewers
Plasma
Summary

This diff is only touches the editing part of the ConfigEditor.

Changes:

  1. Pass config values as the respecting types: int, bool and string. This simplifies config editing in some extent - no more hardcoded options names, that requires quotes around values in gtkrc-2.0 and xsettingsd.conf. Also it simplifies working with GSettings API and helps to transform KDE config options to GTK config options.
  2. Remove redundant \\n symbols, when adding new option in gtkrc-2.0 and xsettingsd.conf
Test Plan
  1. Restart Kded
  2. Test all settings syncing. Specifically boolean ones and enum ones, as the format in configuration files has changed:
  3. Settings.ini uses new booleans (true and false instead of 1 and 0)
  4. Settings.ini, .gtkrc-2.0 and xsettingsd.conf use int value of enum instead of string

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Branch
arcpatch-D27100
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24603
Build 24621: arc lint + arc unit
gikari created this revision.Feb 2 2020, 1:45 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 2 2020, 1:45 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gikari requested review of this revision.Feb 2 2020, 1:45 PM
gikari retitled this revision from Parameterise GTK config editing to [GTK Config] Parameterise GTK config editing.Feb 25 2020, 3:24 PM
cblack requested changes to this revision.Feb 25 2020, 4:38 PM

There's a lot of string concatenation going on. I would replace a lot of the instances where you concat more than 2 strings with QString's arg function for readability purposes.

Will review more thoroughly when I get the time.

This revision now requires changes to proceed.Feb 25 2020, 4:38 PM
This comment was removed by gikari.
gikari updated this revision to Diff 77185.Sat, Mar 7, 9:31 PM

Use arg instead of + + + + + +

gikari updated this revision to Diff 77909.Wed, Mar 18, 12:41 PM

Rebase on master

cblack accepted this revision.Wed, Apr 1, 8:08 PM

Besides one minor over-verbosity issue, this looks like shipit material to me. Drop the QVariant() wrappers (but don't drop the #include) before landing, please.

kconf_update/gtktheme.cpp
42

You don't need to wrap the QStringLiteral with a QVariant here

55–56

Ditto here

This revision is now accepted and ready to land.Wed, Apr 1, 8:08 PM
gikari updated this revision to Diff 79089.Wed, Apr 1, 8:23 PM
gikari marked 2 inline comments as done.
  • Remove explicit QVariant wrappers