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

Add test suite and fix failing tests #74

Merged
merged 6 commits into from
Oct 18, 2024
Merged

Conversation

sellout
Copy link
Contributor

@sellout sellout commented Oct 17, 2024

This does a number of things, so it may be worth reviewing commit-by-commit.

First, it introduces some layered infrastructure:

  1. Buttercup test suites, with some basic tests and other ones to try out different init styles;
  2. Eldev, for Emacs package management;
  3. Nix, for coördination (e.g., running multiple test configurations, building against multiple Emacs distributions); and
  4. garnix, for CI.

They are somewhat independent (e.g., you could rip out the unit tests and keep the rest), but it’s easiest to remove the higher-numbered layers first if they are undesirable. E.g., garnix could be replaced with GitHub workflows (but at the cost of more configuration, which I’m not interested in writing).

The other commits fix the complaints:

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).
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
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-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 LionyxML#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.
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`.
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.)
@sellout
Copy link
Contributor Author

sellout commented Oct 18, 2024

I thought I already made this comment, but maybe I put in it the wrong place …

The CI results for this build are at https://garnix.io/commit/d63ef59c97faf84df0cf76fc83f5bfcb6423ea5d and for my fork in general: https://garnix.io/repo/sellout/auto-dark-emacs

To get CI on this repo, you need to add the app: https://garnix.io/signup

@LionyxML
Copy link
Owner

Another outstand work @sellout.

Let me approve this one into the development branch, so I can see if my setup on garnix worked.

@LionyxML LionyxML merged commit 988121d into LionyxML:development Oct 18, 2024
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

Successfully merging this pull request may close these issues.

2 participants