frewsxcv / rust-dark-light

Rust crate to detect if dark mode or light mode is enabled
https://crates.io/crates/dark-light
73 stars 12 forks source link

Linux support #4

Closed edfloreshz closed 2 years ago

edfloreshz commented 2 years ago

This PR adds support for Linux by implementing the FreeDesktop standard color-scheme key.

In the event this key isn't present in the system, the crate will fallback to traditional ways of figuring out if dark mode is enabled.

This PR is ready to be merged.

Fixes #1

edfloreshz commented 2 years ago

The last commit implemented detect() for many distros, I would appreciate some help testing it.

frewsxcv commented 2 years ago

Is this pull request still in-progress?

edfloreshz commented 2 years ago

Been kinda busy but still in-process.

Converting to draft.

Be-ing commented 2 years ago

This should prefer the new Freedesktop standard and fall back to legacy desktop environment specific methods if that is not available.

edfloreshz commented 2 years ago

I will take a look at it.

edfloreshz commented 2 years ago

I added initial support for FreeDesktop's color-scheme key, I would appreciate some help testing this code as I don't currently have this key present in my system.

edfloreshz commented 2 years ago

This should be ready to merge now 🙂

frewsxcv commented 2 years ago

@edfloreshz @Be-ing I just invited you both as collaborators to this repository. I don't have a Linux box to test any of this on, or really any knowledge of how dark/light mode work on Linux, so feel free to merge this when you feel it's ready to go. Ping me whenever you need a new release published.

edfloreshz commented 2 years ago

I got a 404 when I clicked the invitation 🤔

frewsxcv commented 2 years ago

@edfloreshz What happens if you go here? https://github.com/frewsxcv/rust-dark-light/invitations

edfloreshz commented 2 years ago

Got it! Thanks 🙏🏼

Be-ing commented 2 years ago

I would suggest we test this thing out before we merge it

How about I rebase and clean up the Git history tomorrow and open a new pull request before merging? I'll do some testing on all the supported desktop environments too.

edfloreshz commented 2 years ago

I would suggest we test this thing out before we merge it

How about I rebase and clean up the Git history tomorrow and open a new pull request before merging? I'll do some testing on all the supported desktop environments too.

Sounds good! Let me know if you notice something else.

edfloreshz commented 2 years ago

How about I rebase and clean up the Git history tomorrow and open a new pull request before merging? I'll do some testing on all the supported desktop environments too.

We can squash all the commits into one, edit the commit message and merge right from GitHub, we don't have to open another PR. @Be-ing

I have redacted the following commit message, let me know your thoughts.

* Linux support

This PR implements support for Linux/BSD by using the FreeDesktop Color Scheme Standard.
On supported systems this will be the preferred option, otherwise it will use 
traditional methods to determine the current state of the OS color scheme.

If that's the case, the following distributions have been implemented so far:

- KDE Plasma
- GNOME
- Mate
- Unity
- XFCE
- Cinnamon

The rest of the distros will fall back to light mode.

We will be implementing more of them in the future.

Changelog:
- Adopted FreeDesktop Color Scheme Standard using `zbus`.
- Marked dependencies as Linux/BSD only.
- Implemented `Mode` and created `from` function that takes a bool and returns either `Light` or `Dark`.
- Implemented `check_dconf` for GNOME based Desktop Environments. 
- Implemented `check_file` to read from configuration files for other distributions.

Co-authored-by:

- Corey Farwell <coreyf@rwell.org>
- Funami580 <Funami580@users.noreply.github.com>
- Be <be.0@gmx.com>

If you're okay with this commit message let me know after you're done testing so I can merge.

Be-ing commented 2 years ago

I did a bit more than squashing to one big commit, see #6.

edfloreshz commented 2 years ago

Closing because of #6