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

correct dark detection for gtk3 #26

Open
actionless opened this issue Jul 19, 2022 · 19 comments
Open

correct dark detection for gtk3 #26

actionless opened this issue Jul 19, 2022 · 19 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@actionless
Copy link

import gi
gi.require_version('Gtk', '3.0')
from gi.repository import Gtk


style_context = Gtk.Window().get_style_context()
vals = [0, 0]
for idx, color in enumerate((
        style_context.get_background_color(
            Gtk.StateFlags.NORMAL
        ),
        style_context.get_color(
            Gtk.StateFlags.NORMAL
        )
)):
    for channel in ('red', 'green', 'blue', ):
        vals[idx] += getattr(color, channel)
print(f'Is dark theme: {vals[0] < vals[1]}')
@actionless actionless changed the title correct dark detection for gtk correct dark detection for gtk3 Jul 19, 2022
@actionless
Copy link
Author

in can be also ported to gtk4, but i dont have time

@actionless
Copy link
Author

actionless commented Jul 19, 2022

if gi can't be imported (or can't require version 3) - fallback to the current detection method

@albertosottile
Copy link
Owner

Hi, thanks for this contribution. The idea behind darkdetect is to depend only on packages in the standard library. Up to now this was possible, and so I would like to keep it that way. May I ask you what does not work with the current detection method in your experience? Perhaps we could try to improve that, instead of adding a dependency from gi.

@actionless
Copy link
Author

The idea behind darkdetect is to depend only on packages in the standard library.

#26 (comment)

May I ask you what does not work with the current detection method in your experience?

the current detection simply relies on theme having "Dark" string in its name - that not works for any themes which are not manifesting the level of their darkness via their name

@albertosottile
Copy link
Owner

if gi can't be imported (or can't require version 3) - fallback to the current detection method

I do not think this is how a Python package should work. When you install darkdetect, you expect it to be fully functional, and not having a missing dependency. How is the regular used supposed to know that an extra feature is available if gi is installed in their system? We could make an extra for this dependency, but I fear this will complicate the project structure and distribution too much.

the current detection simply relies on theme having "Dark" string in its name - that not works for any themes which are not manifesting the level of their darkness via their name

This is actually a noble goal, so I understand why you submitted this code. Do you think it would be possible to achieve the same results by e.g. looking at how PyGObject is implemented?

@actionless
Copy link
Author

gi is PyGObject

@albertosottile
Copy link
Owner

I am aware... Let me rephrase: do you think it would be possible to achieve the same results by looking at how gi is implemented?

@actionless
Copy link
Author

you can try copy-pasting code from there and related schemas - but that's not the way how libraries in python ecosystem meant to be used

@actionless
Copy link
Author

you expect it to be fully functional, and not having a missing dependency

the current implementation have the same issue btw - if gsettings executable dependency won't be installed - it also won't be functional

@actionless
Copy link
Author

using gi - and catching ImportError would be the cleanest possible solution

@albertosottile
Copy link
Owner

you can try copy-pasting code from there and related schemas - but that's not the way how libraries in python ecosystem meant to be used

Completely agree, the correct way would be to add PyGObject in the requirements of this project, which is precisely the thing that I do not want to do, for the reasons stated in the README.

the current implementation have the same issue btw - if gsettings executable dependency won't be installed - it also won't be functional

Is this a common scenario? Are there GNOME-based distros that do not include a functional gsettings by default?

using gi - and catching ImportError would be the cleanest possible solution

I would still question how a typical user (that just installs Darkdetect from PyPI) should know that they should also install PyGObject to get an extra or improved feature.

@actionless
Copy link
Author

actionless commented Aug 14, 2022

python package doesn't have any mechanisms to control dependency on gsettings or GNOME, but have mechanism to define an optional dependency on gi

and the main point, detecting dark color should happen by color, not by name

so our discussion is a bit like what you trying to proof what having 5-wheel car is more logical than 4

@albertosottile
Copy link
Owner

albertosottile commented Aug 14, 2022

I am sorry, what would your suggestion be for defining an optional dependency on gi? Catching ImportError does not define any dependency, as the user does not even know that Darkdetect has this dependency in the first place.

As I said before, we could define an extra for this dependency, assuming that there are people interested in this feature. If this is your suggestion, I would consider a PR.

@albertosottile albertosottile added enhancement New feature or request and removed requires user input labels Aug 14, 2022
@actionless
Copy link
Author

https://stackoverflow.com/questions/6237946/optional-dependencies-in-distutils-pip

sure i'll create a PR as soon as we'll get any sort of agreement on the topic - to avoid doing the job which would be discarded

@actionless
Copy link
Author

i mentioned catching ImportError - because it will help to preserve the current behavior if the optional dependency is not installed

@albertosottile
Copy link
Owner

https://stackoverflow.com/questions/6237946/optional-dependencies-in-distutils-pip

Yes, I am well aware, I have been mentioning extras in this issue a few times…

sure i'll create a PR as soon as we'll get any sort of agreement on the topic - to avoid doing the job which would be discarded

I still think this complicates the package significantly, but I would welcome a well-structured PR with the dependency managed as an extra (write a different module for a gi based detection, fall back to https://github.com/albertosottile/darkdetect/blob/master/darkdetect/_linux_detect.py if the extra was not installed, handle module selection in __init__.py).

@Natetronn
Copy link

Is this why Breeze comes back as a Light theme (even though it's very much a dark one) and Adwaita-dark works as it's suppose to?

@albertosottile
Copy link
Owner

Is this why Breeze comes back as a Light theme (even though it's very much a dark one) and Adwaita-dark works as it's suppose to?

I would say so, the current code identifies a theme named 'Breeze' as light.

@albertosottile
Copy link
Owner

In #30 we added an extra to the package: macos-listener, for installing the dependencies necessary for the macOS listener. I am still willing to accept a PR based on @actionless suggestion, providing that the extra dependency on gi is separated in another extra (e.g. gtk-detection).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants