Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set some settings in GSettings as well #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wb9688
Copy link
Contributor

@wb9688 wb9688 commented Mar 2, 2021

In some cases (e.g. on Wayland) GTK 3 reads some settings only from GSettings and not from its configuration file.

Note that there are various versions of gsettings-desktop-schemas, so it's needed to check if a setting is actually available.

I'm currently not setting toolbar-style and toolbar-icons-size yet, because in some versions it's a string choice and in other versions it's an enum (so it's a bit more work to implement), plus only small and large could be chosen for toolbar-icons-size while LXAppearance has more options than that.

Edit: the underlying data type of enum is also string, so that actually doesn't matter.

@wb9688 wb9688 marked this pull request as draft March 16, 2021 10:51
@wb9688 wb9688 marked this pull request as ready for review March 16, 2021 18:20
@wb9688
Copy link
Contributor Author

wb9688 commented Mar 16, 2021

@LStranger: This should be good to go now.

@PRESFIL
Copy link

PRESFIL commented Dec 26, 2021

Upvote, because QGnomePlatform reads GTK theme only from Gsettings.
This will allow lxappearance to be used in modern environments and switch GTK
and Qt themes synchronously from lxappearance.

@@ -2125,24 +2125,12 @@
<column type="gchararray"/>
</columns>
<data>
<row>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean those extra choices should not be used or cannot be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not in the old config file, can not in the new GSettings.

@PRESFIL
Copy link

PRESFIL commented Jun 5, 2022

Any updates on this?

@ib ib added the question Further information is requested label Sep 8, 2023
@ib
Copy link
Member

ib commented Sep 8, 2023

@wb9688

  • If the GSettings are for GTK 3, why don't we restrict it to that?
  • If there are only two icon sizes available for the GSettings, wouldn't it make more sense to empty the combo box in the program and fill it with those two instead of limiting the icon size choices for everyone?

@wb9688
Copy link
Contributor Author

wb9688 commented Sep 8, 2023

@ib

  1. Because LXAppearance is for configuring both GTK 2 and GTK 3 themes, independent of what GTK it is compiled with. LXAppearance currently also already writes the GTK 3 config file at https://github.com/lxde/lxappearance/blob/master/src/lxappearance.c#L373, however GTK 3 doesn't always use the settings from the config file and in some cases, e.g. on Wayland, only looks at the GSettings.
  2. It reused the icon size enum, so it was possible to use the other options as toolbar icon size, but only small and large toolbar were intended as icon sizes for toolbar icons. With GSettings they have limited it to only those 2 that were intended to be used for this, so I thought it would make sense to do this.

As for my other PRs that break GTK 2 compilation according to you, I hope I have time this weekend to look into it.

@ib
Copy link
Member

ib commented Sep 8, 2023

@wb9688

  1. My point is: If GSettings is only needed for GTK 3, then the code block should only be compiled for GTK 3.
  2. I understand your intention, but it would be nicer if things didn't change, they don't have to with GTK 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants