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

Use xdg-desktop-portal on Linux #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JakobDev
Copy link

The xdg-desktop-portals are a set of DBus Interfaces that are implemented by every Desktop Environment. They are also available in sandboxed Environments like Flatpak or Snap. They provide a standardized way to interact with the DE. You can detect the darkmode using portals, so you no longer need gsettings, but I kept the code for backwards compatibility.

This PR adds jeepney as Dependency for Linux. I also bumped the required Python version to 3.7, as jeepney requires this and 3.6 is out of support anyway.

@albertosottile
Copy link
Owner

Hi, thanks for submitting this. The core philosophy of darkdetect is that

this package does not depend on other modules or packages that are not already included in standard Python distributions

as stated in the beginning of the README.

That being said, in #30 we opened up the possibility to add external dependencies as setuptools extras. Therefore, I would be willing to review this PR is you could restrict this new dependency in a separate, appropriately named extra (e.g. linux-portals). Users that are willing to install the extra (e.g. with pip install darkdetect[linux-portals]) will get the new functionality. On the other hand, the code will have to fall back to the existing detection system in case the extra is not installed.

@JakobDev
Copy link
Author

I understand, that depending on other packages is something, you not want. I also try to keep the decencies as low as possible, but in this case, we have to use DBus. The current solution is hacky and may not work in every Environment while the solution provided here should work. Using optional dependencies here, is IMHO a bad Idea. Many people will just use pip install darkdetect, especially when they program cross platform. darktedect may be also somewhere in the dependency tree, when using a GUI library, so we should provide a working solution for everyone.

Another way to solve this, would maybe using the gdbus cli. You can use it like this:

gdbus call --session --dest org.freedesktop.portal.Desktop --object-path /org/freedesktop/portal/desktop --method org.freedesktop.portal.Settings.Read org.freedesktop.appearance color-scheme

It should be installed on most Linux ditros, but there is no guarantee. It will output this:

(<<uint32 2>>,)

2 is the number that we are interested. As you can see, the output is formatted. I don't know if this format will change the format in the future. That's the reason I haven't use it. There are other clis for dbus as well, but they are not installed on so many systems as gdbus.

@albertosottile
Copy link
Owner

Many people will just use pip install darkdetect

I apologize, but I disagree. The main users of darkdetect are other programmers willing to include this feature in their software. Therefore, they are in full control of their dependency trees, and they can choose the best flavor of darkdetect they need/they would like to install.

Conversely, adding a mandatory dependency would force every user (in this case on Linux) to include and distribute the dependency in their software. This is not something to take lightly, especially when the software is frozen (with e.g PyInstaller). Specifically, I do not know how well jeepney behaves when frozen with PyInstaller, or what is its licensing model. For some people, having a slim package with a permissive license and no extra dependencies is critical. I hope you understand my reasoning. In any case, you are free to fork this package anytime.

The current solution is hacky and may not work in every Environment

I completely agree, but the current solution was also proposed by other users and up to now it seems they were able to somehow benefit from having it.

Another way to solve this, would maybe using the gdbus cli.

If you want, you can attempt to add this as another detection method, and fall back to the existing method(s) in case there are issues (e.g. missing CLI tool or unexpected output format).

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

Successfully merging this pull request may close these issues.

None yet

2 participants