“Loading a theme can run Lisp code. Really load? (y or n)” breaks Doom Emacs initialization #64
Comments
|
Oh dang! Sorry we broke it for you @ehamberg. I managed to set doom here and loading with this extra argument did not fixed the issue. As a temporary fix while I find some time to proper debug this issue, could you try: (after! doom-ui
(setq custom-safe-themes t) ;; <<<---- ADD THIS
;; set your favorite themes
(setq! auto-dark-themes '((doom-one) '(doom-one-light)))
(auto-dark-mode))Tell me if it 'solves' it for you 😄 |
|
Added this note to README via #65. |
It did!1 Thanks! I couldn't get Footnotes
|
|
@ehamberg Sorry to have caused you trouble. Can you show me how you were setting The removal of the optional argument to I made an effort to keep the old approach working alongside the new. After figuring out which var to pull the list of themes from, the logic is shared – so I’m surprised the (setq! auto-dark-themes '((doom-one) '(doom-one-light)))should be (setq! auto-dark-themes '((doom-one) (doom-one-light)))Did you maybe copy that bug? If not, can you show me how you tried setting @LionyxML I can address updating the docstrings and fixing my README typo. |
I probably did copy that bug, because |
|
Thanks for offering to change the README @sellout. Yeah, the typo seemed to work for me because it defaulted to Emacs default theme (light) and gave me the false impression it was alright, lol. I was messing again with At least for my version: GNU Emacs v30.0.91 nil
Doom core v3.0.0-pre grafted, HEAD -> master, origin/master, origin/HEAD 2e5307e 2024-09-14 13:08:00 -0700
Doom modules v24.10.0-pre grafted, HEAD -> master, origin/master, origin/HEAD 2e5307e 2024-09-14 13:08:00 -0700The The working example would be something like: ;; packages.el
(package! auto-dark)
;; config.el
(after! doom-themes
(setq custom-safe-themes t) ;; as discussed on https://github.com/LionyxML/auto-dark-emacs/issues/64
(setq auto-dark-themes '((doom-one) (doom-gruvbox)))
(auto-dark-mode))Could you please validate if this snippet also works for you @ehamberg? |
|
Just adding this here in case anyone finds it useful. I don't use Doom, but when I encountered this issue in vanilla after upgrading and saw the fix in the documentation, I decided to adapt it slightly using advice so that (use-package auto-dark
:if (display-graphic-p)
:after solarized-theme
:diminish
:custom
(auto-dark-themes '((solarized-dark wombat) (solarized-light leuven)))
(auto-dark-polling-interval-seconds 5)
(auto-dark-allow-osascript (eq system-type 'darwin))
:init
(defun advice/auto-dark--enable-themes (auto-dark--enable-themes &rest args)
"Set `custom-safe-themes' to t when calling AUTO-DARK--ENABLE-THEMES with ARGS.
See URL `https://github.com/LionyxML/auto-dark-emacs/issues/64' for more information."
(let ((custom-safe-themes t)) (apply auto-dark--enable-themes args)))
(advice-add 'auto-dark--enable-themes :around 'advice/auto-dark--enable-themes)
(auto-dark-mode))This was just a little more readable for me and has two added benefits: i) it shows the advice docstring when looking up |
Not really. It seems to only work after one dark mode toggle. So with the following config… (after! doom-themes
(setq custom-safe-themes t) ;; as discussed on https://github.com/LionyxML/auto-dark-emacs/issues/64
(setq auto-dark-themes '((leuven-dark) (leuven)))
(auto-dark-mode))… I first get the doom-one (default, I guess) theme, then if I enable the system's dark mode, I get the leuven-dark theme, then if I disable the system's dark mode I get leuven. This is the actual, working config I use in my (use-package! auto-dark
:config
(setq custom-safe-themes t)
(setq! auto-dark-themes '((doom-bluloco-dark) (doom-bluloco-light)))
(auto-dark-mode)) |
|
Thanks for testing @ehamberg! Well, this also works for me (+ the entry on packages.el). Maybe this is going to be our new DoomEmacs snippet? |
Ohhh dang, I missed this message! Humm, so this is also happening on Vanilla Emacs? Maybe we should add this 'fix' on the Readme for all cases. What do you think @sellout ? |
Yeah, But I think if you don’t just want to set it to (use-package auto-dark
:custom
(auto-dark-themes '((solarized-dark wombat) (solarized-light leuven)))
(custom-safe-themes
'("c8cfb034378af37e278fbf1d7cc6584131b686650fb0503d78c59817310aee54" ; leuven
"52c3d95f52f9e2889576bd7500fbbc7d3d56f1f718d734d87f02e9cc309eaa77" ; solarized-dark
"ba5fedf9df7a51454f21f100479c3cf562b66c919be41cf2bfdc088330e77ed4" ; solarized-light
"ff6aecadd496de713fce479ee3aca55359f2af8b1955d61c9e05363e4d97d8f4" ; wombat
)))Where each string is the SHA-256 hash of the theme file in question (see The Emacs Manual, §50.1.7 Custom Themes). NB: I just made up those SHAs, they’re not the right ones for those themes. One other possibility is adding something like |
|
Ok, I’ve thought about this more than I should. First, I was wrong to suggest Second, as we’ve seen here, it might be the case that the first time a theme is loaded is by Auto Dark – either at Emacs initialization, or when the @ehamberg has an issue where Doom Emacs perhaps doesn’t finish initialization after these prompts, but that sounds like it may be a Doom Emacs issue (which Auto Dark should call out, but shouldn’t require code changes here). @mepcotterell has a similar issue with vanilla Emacs. I use a pretty vanilla Emacs as well, but can’t seem to replicate anything like this. @mepcotterell – do you the redraw failure that @ehamberg talks about, or by “same issue” do you mean only that it prompts for whether you should load the theme? (in which case, I think answering yes/yes is the best solution) @ehamberg I’m curious exactly what the interaction is here (so I can be clear in the documentation, etc.) You start Doom Emacs, and during initialization it prompts “Really load?” … after this, can you respond to the prompt, or are you not given the opportunity? If you could respond, does the failure to redraw happen regardless of the response, or only with a particular response? And by “failed to redraw”, do you thing the rest of initialization didn’t happen, or was it specifically just redrawing, and other packages, etc. loaded normally? If you try without Auto Dark, but instead just (after! doom-themes
(custom-set-variables '(custom-enabled-themes (doom-bluloco-dark))))Does that still prompt you, and does the prompt still break initialization? If it still breaks initialization, then I think we have an issue to open against Doom Emacs. @mepcotterell If you’re getting an initialization failure, too, I’d love for you to answer those same questions. @ehamberg has another issue where the first time through the wrong theme loads. I had something sort of like this happen to me, where during initialization I’ll adjust this based on responses, but here’s what I’m thinking now:
Thoughts? |
|
So many people helping! 😍 Nice! @ehamberg, for me, it works with both @sellout, nice point of view. For me, 1 and 2 are no-brainers; I'd definitely go for them. As for 3 and 4, I like the idea of auto-dark taking control of mismatched settings. It's not uncommon to see some In the meantime, I'll try setting up some local testing environments on macOS/Linux, at least with vanilla Emacs, my own config, and Doom. |
|
Ah, I made all these changes last night. I should have put up at least a draft PR. I’ll get one up tonight, and people can tell me what they think (and maybe test out the branch). |
|
@sellout I was getting the confirmation prompt every time I started Emacs, despite the proper hashes being present in If I defer calling I know that I can set I’m personally okay with auto-dark assuming the themes it loads are safe since I get to decide what those themes are. If other elisp code maliciously adjusts the load path so that a different theme with the same name is loaded when I’m currently considering what compromise I’m comfortable with, but an ideal solution for me mitigates or prevents the security issue, lets customize continue to manage the value of |
|
Ah, thanks @mepcotterell, that context really helps.
Absolutely – I like to load my In any case, I’ll join you in thinking about what might be a good solution for this case. |
|
I put up #67, which addresses at least some of the concerns here. Didn’t officially say it “fixes” this issue, as there are a lot of different things going on here. |
😬
Yeah, that could definitely be.
I get the prompt, but nothing but the bottom bar (“modline”?) updates: CleanShot.2024-09-25.at.11.48.59.mp4
It does not prompt, and stuff (modulo auto dark, naturally) works.
Even if I only have this in my (after! doom-themes
(setq custom-safe-themes t) ;; as discussed on https://github.com/LionyxML/auto-dark-emacs/issues/64
(setq auto-dark-themes '((leuven-dark) (leuven)))
(auto-dark-mode))…it still uses the default (doom-one) theme until a change of the system's dark mode happens. It does load the leuven theme, though, but doom-one is used until I toggle to dark mode (then it switches to leuven-dark) and then back to non-dark mode (then it switches to leuven). |
|
@sellout Thanks for all your work on this. I really appreciate being able to have a conversation about it.
I don't have any stats related to this, but my gut tells that loading it early is less typical than loading it near the end based on where customize puts
I typically migrate package-specific custom variables into a package's Finally, since In my case, I'm either going to stick with the advice I suggested earlier or end up being okay with deferring auto-dark-mode until after initialization is complete. I will try to check out the PR if I find some time to see if your changes make any of this easier. |
|
@sellout You may also want to consider how all of this impacts users who upgrade their themes to newer versions from time to time since that changes a theme's hash. |
I agree that it’s great to discuss this stuff, I’m certainly learning things, despite being an Emacs user for 20+ years. So, first I want to say that while I have Opinions™️ here, this is, of course, @LionyxML’s project, and I’m happy to implement whatever solution they want or stop this discussion if it’s just too much. Second, one of my opinions, that I don’t think I’ve made explicit here yet, but has definitely informed my comments & PRs is that I don’t think a library should make a decision about It is interesting (and my Emacs config is so ancient now that I often forget the truly vanilla behavior) that Customize puts things at the end of In the spirit of @mpgcottel’s list of things they want to achieve with the config, I want
The Some different approaches to configuration:
I feel like Emacs wants users to let it manage the set of So, I don’t yet see a solution that satisfies my criteria2 (which as I mentioned are of secondary importance on this repo).
Yeah, this is quite annoying.
Yes – I think this is largely out of the scope of this project, aside from the README/docs informing the user of I.e., I feel like if Auto-Dark has a variable like Footnotes
|
|
@ehamberg And I haven’t forgotten about your issues – just need to dig in more about what’s going on there. |
|
Nice analysis @sellout ! Thanks for sharing your stream of thought. Well, it is as much of a me package as it is from all that uses it and improves it, so ideas are always welcome. Regarding these goals:
I think they are nice goals, user should always be aware of any safe options being manipulated. That said, as long as auto-dark can provide the features and Emacs is properly behaving (I've seen many magic elisp executions where messages do not appear or Emacs does not render something on screen while some other function is finished, I do think some behaviors on Emacs updates to the screen are not well documented, or I haven´t stumbled on them yet...). About |
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.
* 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.)
After #57 was merged, I get
Loading a theme can run Lisp code. Really load? (y or n)when I start Emacs which seems to break something with the Doom Emacs initialization and leads to the window not being redrawn.(Is this because it now calls
(load-theme theme)instead of(load-theme theme t)?)The text was updated successfully, but these errors were encountered: