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

[FR] Fallback in case no detection mechanism could be determined #66

Open
edgar-vincent opened this issue Sep 18, 2024 · 2 comments
Open

Comments

@edgar-vincent
Copy link

edgar-vincent commented Sep 18, 2024

Hi!

Thanks for this excellent package. I recently used my usual Emacs configuration, which enables auto-dark, in a Docker container. Of course, auto-dark failed with this error: Could not determine a viable theme detection mechanism!, which is to be expected.

The problem is that this interfered with Emacs' initialisation process. Would it be possible to switch to a default fallback theme if auto-dark fails and skip the error?

If you're interested, I'd be happy to provide a PR. Actually, it could be an extension of #62.

Thanks again!

EV

@LionyxML
Copy link
Owner

Thanks for reaching out @edgar-vincent.

Indeed it is a point of improvement, I'll keep this is mind. Auto-dark could 'not load' without breaking Emacs boot 😅

@sellout
Copy link
Contributor

sellout commented Sep 25, 2024

Rough thought is to

  1. replace (error …) with (lwarn 'auto-dark :error …),
  2. add something to the messages about having to manually set auto-dark-detection-method, then
  3. have unregister-/register-change-listener fire on changes to auto-dark-detection-method.

Right now the change listener is only registered when the mode is enabled, so step 3 makes it a little more ergonomic for users to post-hoc set the change listener.

I‘m happy to make these changes to #62.

sellout added a commit to sellout/auto-dark-emacs that referenced this issue Oct 17, 2024
Previously, failing to “determine a viable theme detection mechanism” would
error, preventing the rest of `auto-dark-mode` setup from running. This is now a
warning, and you are effectively left in “manual” mode.

This is technically a fix for LionyxML#66, but doesn’t address all the related changes
there. Those will be addressed in LionyxML#62. It also partially addresses LionyxML#73 by adding
`auto-dark-toggle-appearance`.
LionyxML pushed a commit that referenced this issue Oct 18, 2024
* Set up build & test infrastructure

This has a few layers:
1. Eldev, for Emacs package management;
2. Nix, for coördination (e.g., running multiple test configurations, building
   against multiple Emacs distributions); and
3. garnix, for CI.

Each should work without the ones after it, so it is somewhat modular. E.g.,
garnix could be replaced with GitHub workflows (but at the cost of more
configuration).

* Appease the linters

There are a couple things in here that should be changed:
- don’t use `fboundp` on every invocation
- don’t set `fill-column` to 167

* Add a test suite

There are three pieces to this
- “standard” unit tests (currently fairly minimal)
- initialization tests that check how various styles of user init behave
- a library for simulating the Emacs init process

* Pre-load themes & update state when vars set

Pre-loading themes helps shift `custom-safe-theme` interactions to when the user
sets the variable instead of during initialization or mode change, but this
isn’t a full fix for #64, since it doesn’t address the conundrum of
`custom-safe-themes` being set after `auto-dark-mode` is enabled.

Updating the state when the variables are set fixes the “it seems to only work
after one dark mode toggle” issue (which especially crops up when variables are
customized after the mode is enabled). Users of the timer likely don’t notice
this, as it only takes five seconds for Auto-Dark to fix the state, but the
pub/sub detection methods wouldn’t otherwise update until the next mode change.

* Set themes even if detection mechanism fails

Previously, failing to “determine a viable theme detection mechanism” would
error, preventing the rest of `auto-dark-mode` setup from running. This is now a
warning, and you are effectively left in “manual” mode.

This is technically a fix for #66, but doesn’t address all the related changes
there. Those will be addressed in #62. It also partially addresses #73 by adding
`auto-dark-toggle-appearance`.

* Defer setting old variables

Since the old `auto-dark-dark/light-theme` variables have defaults, a
traditional configuration (enabling Auto-Dark before customizing vars) can lead
to a `default` → `auto-dark-dark/light-theme` → `auto-dark-theme` “flicker”
sequence during Emacs initialization.

This avoids that by deferring initialization of the old variables until
`after-init-mode`, so they only affect the display if both `auto-dark-themes`
and `custom-enabled-themes` are `nil`.

The consequence is that users of the old variables may have a slightly longer
delay until the initial Auto-Dark theme appears. (And also that Auto-Dark has a
bit more defensive code to ensure it doesn’t try to set themes before enough is
initialized.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants