Keep existing configurations' strings               
ClosedPublic

Authored by gikari on Jun 1 2019, 4:23 PM.

Details

Summary

Gtk configurator has been replacing .gtkrc-2.0 config file with it's own settings, discarding present file contents. Now it only modifies configuration strings, or add them if they are not present at the beginning of the file.

This patch also removes redundant configurations, such as including theme from /usr/share/themes gtkrc's, "user-font" style, widget_class and unneeded symlink .gtkrc-2.0-kde4. Those options do not seem needed, as gtk2 applications look the same without these options without any regressions. This was tested on Gimp, Inkscape and BleachBit apps on Manjaro, OpenSUSE and KDE Neon distributions.

As for gtk3 ini file - this is not reproducible anymore, any line, which does not hold parameter changeable in configurator, remains in settings.ini.

BUG: 322797
BUG: 354963
BUG: 342320
FIXED-IN: 5.17.0

Test Plan

Insert some lines in .gtkrc-2.0 file, that do not represent settings, that configurator changes, check if they are still present after applying new settings.

Check if gtk2 apps look identical with or without .gtkrc-2.0-kde4 symlink, with or without similar lines in .gtrkrc-2.0:

include "/usr/share/themes/Breeze/gtk-2.0/gtkrc"
style "user-font" 
{
	font_name="Noto Sans Regular"
}
widget_class "*" style "user-font"

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
gikari created this revision.Jun 1 2019, 4:23 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 1 2019, 4:23 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gikari requested review of this revision.Jun 1 2019, 4:23 PM
gikari edited the test plan for this revision. (Show Details)Jun 2 2019, 8:14 AM
gikari added a reviewer: apol.
gikari edited the summary of this revision. (Show Details)Jun 2 2019, 10:07 AM
gikari edited the summary of this revision. (Show Details)
apol added a comment.Aug 5 2019, 10:45 AM

Is this only necessary for gtk2?

src/appearancegtk2.cpp
125

const?

136

This constructor isn't necessary.

138

This constructor isn't necessary

143

This constructor isn't necessary.

gikari updated this revision to Diff 63142.Aug 5 2019, 6:12 PM

Remove redundant QString constructors and apply const modifier to QRegularExpression

gikari marked 4 inline comments as done.Aug 5 2019, 6:16 PM
In D21524#506866, @apol wrote:

Is this only necessary for gtk2?

Yes. Gtk3 settings.ini custom entries remain untouchable, when settings are applied.

apol accepted this revision.Aug 24 2019, 2:30 AM

Let's see how this goes. Do you have push powers?

This revision is now accepted and ready to land.Aug 24 2019, 2:30 AM
In D21524#517780, @apol wrote:

Do you have push powers?

No, I don't have.

ngraham edited the summary of this revision. (Show Details)Aug 24 2019, 1:06 PM
This revision was automatically updated to reflect the committed changes.

This breaks the unit test locally as well as on build.kde.org, eg.: https://build.kde.org/job/Plasma/job/kde-gtk-config/job/kf5-qt5%20SUSEQt5.12/51/console and https://build.kde.org/job/Plasma/job/kde-gtk-config/job/kf5-qt5%20FreeBSDQt5.13/5/console

Start 2: configsavetest

2: Test command: /var/tmp/paludis/build/kde-kde-gtk-config-scm/work/build/tests/configsavetest
2: Test timeout computed to be: 10000000
2: *** Start testing of ConfigSaveTest *
2: Config: Using QtTest library 5.13.1, Qt 5.13.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 9.2.0)
2: PASS : ConfigSaveTest::initTestCase()
2: QWARN : ConfigSaveTest::testGtk2() There was unable to write the .gtkrc-2.0 file
2: FAIL! : ConfigSaveTest::testGtk2() 'instance->saveSettings(pathA)' returned FALSE. ()
2: Loc: [/var/tmp/paludis/build/kde-kde-gtk-config-scm/work/kde-gtk-config-scm/tests/configsavetest.cpp(63)]
2: PASS : ConfigSaveTest::testGtk3()
2: PASS : ConfigSaveTest::cleanupTestCase()
2: Totals: 3 passed, 1 failed, 0 skipped, 0 blacklisted, 3ms
2:
* Finished testing of ConfigSaveTest *
2/2 Test #2: configsavetest ...................
*Failed 0.02 sec