BunsenLabs / bunsen-utilities

https://pkg.bunsenlabs.org/debian/pool/main/b/bunsen-utilities/
GNU General Public License v3.0
31 stars 21 forks source link

bl-reload-gtk23: Implement optional sync of effective GTK3 config to xsettingsd before reloading xsettingsd #71

Closed ghost closed 4 years ago

ghost commented 4 years ago

As discussed in https://forums.bunsenlabs.org/viewtopic.php?id=6786, here is a quick proof of concept for our use case.

The final thing to decide is whether to make the sync of the config mandatory when reloading gtk3 theming or not. I think it should be mandatory as otherwise the functionality might not work as expected. However, the use case of allowing the user to re-apply settings that are already in the xsettingsd config file, for example a manually created xsettingsd config file, though this does not seem to make much sense except when settings.ini is no longer relevant.

ghost commented 4 years ago

I'll update the help texts tomorrow. I see some typos and of course the phrasing is no longer correct if this gets merged. Would then also need to add xsettingsd to debian/control:Depends or Recommends.

johnraff commented 4 years ago

I've been testing this - after a long hiatus - and can report:

Three GTK properties are being incorrectly identified as integer types, not booleans, and failing to map to xsettings: gtk-cursor-theme-size=0 gtk-xft-antialias=1 gtk-xft-hinting=1 all gave rise to this debug message: WARNING bl-reload-gtk23 sync_gtk3() : Ignoring source key of unhandled type: 0 (<class 'int'>) even though gtk-enable-event-sounds=0 is correctly identified as a boolean:

DEBUG bl-reload-gtk23 sync_gtk3() : Mapping FROM gtk-enable-event-sounds = 0 (<class 'bool'>)
2020-09-12 16:11:10,362 
DEBUG bl-reload-gtk23 sync_gtk3() :         TO   Net/EnableEventSounds 0 (<class 'int'>)

I added three new keys to the parse list in bl-reload-gtk23:

      "gtk-font-name"                    : "Gtk/FontName",
      "gtk-cursor-theme-name"            : "Gtk/CursorThemeName",
      "gtk-cursor-theme-size"            : "Gtk/CursorThemeSize",

but that doesn't seem to be a problem in itself. The first two keys are being parsed correctly.

johnraff commented 4 years ago

The final thing to decide is whether to make the sync of the config mandatory when reloading gtk3 theming or not. I think it should be mandatory as otherwise the functionality might not work as expected. However, the use case of allowing the user to re-apply settings that are already in the xsettingsd config file, for example a manually created xsettingsd config file, though this does not seem to make much sense except when settings.ini is no longer relevant.

Make the gtk3 settings.ini > xsettingsd sync by default, with a cli option to ignore settings.ini? I think in the vast majority of use cases sync would be wanted.

ghost commented 4 years ago

@johnraff I've pushed changes:

This version is not final, some could be written cleaner, but ought to be good enough for testing.

johnraff commented 4 years ago

Thanks - this seems to be well on the way now, though a couple of small issues remain:

1) The “gtk-toolbar-icon-size” and “gtk-toolbar-style” properties are deprecated and will cause the script to exit with an error if mapping is attempted. I suggested commenting the lines, but they could just be removed.

2) Some booleans seem to be mapped to `0', regardless of their GTK value. eg:

2020-10-08 16:54:19,784 DEBUG bl-reload-gtk23-twoion sync_gtk3() : Mapping FROM gtk-enable-event-sounds = 1 (<class 'bool'>)
2020-10-08 16:54:19,784 DEBUG bl-reload-gtk23-twoion sync_gtk3() :         TO   Net/EnableEventSounds 0 (<class 'int'>)
ghost commented 4 years ago

@johnraff

johnraff commented 4 years ago

the "truthy" state is represented as the string '1' whereas the code only handled the case of the truthy value being 'true'. The ultimate fix would be to know all values that GTK would consider truthy but tonight I couldn't find this within 10 minutes so for know I replaced it with a simple list ('1' and 'true', and only these strings counting as 'true'). Who knows what GNOME software used to put into settings.ini (like possibly 'TRUE', 'True', 'yes', 'YES').

not clear from a quick search.

Confirmed.

But while all web reference to settings.ini that I could find used 1, the GTK3 Settings reference does use upper-case TRUE, so maybe that could be added to the TRUTHY list?

Some other (than the toolbar* already removed) properties referenced by bl-reload-gtk23 are now deprecated according to that page:

   "gtk-button-images"                : "Gtk/ButtonImages", # This setting is deprecated. Application developers control whether a button should show an icon or not, on a per-button basis. If a GtkButton should show an icon, use the “always-show-image” property of GtkButton, and pack a GtkImage inside the GtkButton
   "gtk-can-change-accels"            : "Gtk/CanChangeAccels", # This setting is ignored.
   "gtk-color-palette"                : "Gtk/ColorPalette", # Only used by the deprecated color selector widget.
   "gtk-color-scheme"                 : "Gtk/ColorScheme", # You can still set this property, but it will be ignored.
   "gtk-icon-sizes"                   : "Gtk/IconSizes", # This setting is ignored.
   "gtk-im-preedit-style"             : "Gtk/IMPreeditStyle", # This setting is ignored.
   "gtk-im-status-style"              : "Gtk/IMStatusStyle", # This setting is ignored.
   "gtk-menu-bar-accel"               : "Gtk/MenuBarAccel", # This setting is ignored.
   "gtk-menu-images"                  : "Gtk/MenuImages", # This setting is deprecated. Application developers control whether or not a GtkMenuItem should have an icon or not, on a per widget basis. Either use a GtkMenuItem with a GtkBox containing a GtkImage and a GtkAccelLabel, or describe your menus using a GMenu XML description
   "gtk-scrolled-window-placement"    : "Gtk/ScrolledWindowPlacement", # This setting is ignored

It might avoid future trouble to remove those too?

Running bl-reload-gtk23-twoion --debug-gtk-mappings reported two other properties as not existing:

debug_gtk_mappings() : gtk-overlay-scrolling             = ****************** does not exist **************************
debug_gtk_mappings() : gtk-session-bus-id                = ****************** does not exist **************************

I don't know, however, if they might exist on other systems?

But regardless of the above two suggestions about deprecated properties, the script tests out OK on my BunsenLabs Lithium system, doing its job well.

ghost commented 4 years ago

the "truthy" state is represented as the string '1' whereas the code only handled the case of the truthy value being 'true'. The ultimate fix would be to know all values that GTK would consider truthy but tonight I couldn't find this within 10 minutes so for know I replaced it with a simple list ('1' and 'true', and only these strings counting as 'true'). Who knows what GNOME software used to put into settings.ini (like possibly 'TRUE', 'True', 'yes', 'YES').

not clear from a quick search.

Confirmed.

But while all web reference to settings.ini that I could find used 1, the GTK3 Settings reference does use upper-case TRUE, so maybe that could be added to the TRUTHY list?

The uppercase TRUE is so because defined constants in GTK land are uppercase. Doesn't hurt to add it to the list anyway.

Some other (than the toolbar* already removed) properties referenced by bl-reload-gtk23 are now deprecated according to that page:

   "gtk-button-images"                : "Gtk/ButtonImages", # This setting is deprecated. Application developers control whether a button should show an icon or not, on a per-button basis. If a GtkButton should show an icon, use the “always-show-image” property of GtkButton, and pack a GtkImage inside the GtkButton
   "gtk-can-change-accels"            : "Gtk/CanChangeAccels", # This setting is ignored.
   "gtk-color-palette"                : "Gtk/ColorPalette", # Only used by the deprecated color selector widget.
   "gtk-color-scheme"                 : "Gtk/ColorScheme", # You can still set this property, but it will be ignored.
   "gtk-icon-sizes"                   : "Gtk/IconSizes", # This setting is ignored.
   "gtk-im-preedit-style"             : "Gtk/IMPreeditStyle", # This setting is ignored.
   "gtk-im-status-style"              : "Gtk/IMStatusStyle", # This setting is ignored.
   "gtk-menu-bar-accel"               : "Gtk/MenuBarAccel", # This setting is ignored.
   "gtk-menu-images"                  : "Gtk/MenuImages", # This setting is deprecated. Application developers control whether or not a GtkMenuItem should have an icon or not, on a per widget basis. Either use a GtkMenuItem with a GtkBox containing a GtkImage and a GtkAccelLabel, or describe your menus using a GMenu XML description
   "gtk-scrolled-window-placement"    : "Gtk/ScrolledWindowPlacement", # This setting is ignored

It might avoid future trouble to remove those too?

Running bl-reload-gtk23-twoion --debug-gtk-mappings reported two other properties as not existing:

debug_gtk_mappings() : gtk-overlay-scrolling             = ****************** does not exist **************************
debug_gtk_mappings() : gtk-session-bus-id                = ****************** does not exist **************************

I don't know, however, if they might exist on other systems?

But regardless of the above two suggestions about deprecated properties, the scripts tests out OK on my BunsenLabs Lithium system, doing its job well.

I left in the deprecated ones, but removed the ignored ones. I suppose GTK will just change to ignoring deprecated properties at some point.

debug_gtk_mappings() : gtk-overlay-scrolling = ** does not exist ** debug_gtk_mappings() : gtk-session-bus-id = ** does not exist **

This means that values we could map based on documentation don't have a type in GTK settings in the current version of GTK. For example, on my system with gtk 3.24.23 right now, I only get this one report:

2020-10-14 22:00:10,941 INFO bl-reload-gtk23 debug_gtk_mappings() : gtk-session-bus-id                = ****************** does not exist **************************

and gtk-overlay-scrolling exists in my gtk3 version, so it's part of a newer gtk release. If these settings are found in settings.ini on systems where gtk reports it doesn't know them, the script used to abort; now I've added a commit which will just skip over unknown keys in settings.ini.

Good.

Allow me to go over the code later just once more to clean it up and then I guess we're done with this project!

johnraff commented 4 years ago

Excellent! Please feel free to merge this into lithium whenever you're ready.