auto-dark-emacs icon indicating copy to clipboard operation
auto-dark-emacs copied to clipboard

“Loading a theme can run Lisp code. Really load? (y or n)” breaks Doom Emacs initialization

Open ehamberg opened this issue 1 year ago • 23 comments

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)?)

ehamberg avatar Sep 14 '24 18:09 ehamberg

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. What I ended up doing is forcing Emacs to recognize custom themes as safe.

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 😄

LionyxML avatar Sep 15 '24 19:09 LionyxML

Added this note to README via #65.

LionyxML avatar Sep 15 '24 20:09 LionyxML

Tell me if it 'solves' it for you 😄

It did![^1] Thanks!

I couldn't get auto-dark-themes to work though, but my old, trusty (but now deprecated 😬) settings still works:

  (setq auto-dark-light-theme 'doom-bluloco-light)
  (setq auto-dark-dark-theme 'doom-bluloco-dark)

[^1]: But I had to hunt down to places where the value of custom-safe-themes was persisted, first, though. It was saved at the bottom of my config.el and in a custom.el file in ~/.config/doom.

ehamberg avatar Sep 16 '24 11:09 ehamberg

@ehamberg Sorry to have caused you trouble. Can you show me how you were setting auto-dark-themes?

The removal of the optional argument to load-theme should have been called out more clearly, or perhaps made in a second PR – I apologize. But I think it is the correct behavior, and using custom-safe-themes is the right way to silence the confirmation (but it should be called out in the README & docstrings – thanks @LionyxML for addressing my oversight there so quickly).

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 auto-dark-themes isn’t working for you. I did notice I have a typo in the README:

(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 auto-dark-themes?

@LionyxML I can address updating the docstrings and fixing my README typo.

sellout avatar Sep 17 '24 14:09 sellout

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 auto-dark-themes isn’t working for you. I did notice I have a typo in the README:

(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 auto-dark-themes?

I probably did copy that bug, because (setq! auto-dark-themes '((doom-one) (doom-one-light))) worked. Thanks!

ehamberg avatar Sep 18 '24 06:09 ehamberg

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 doomemacs today and I got something strange happening.

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 -0700

The after! doom-ui is actually not working, it must be set to after! doom-themes.

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?

LionyxML avatar Sep 19 '24 22:09 LionyxML

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 custom-safe-themes is only set to t temporarily when auto-dark--enable-themes is called. Here the use-package declaration that I use:

(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 auto-dark--enable-themes using describe-function; and ii) it restores custom-safe-themes automatically, assuming lexical binding is enabled.

mepcotterell avatar Sep 20 '24 11:09 mepcotterell

Could you please validate if this snippet also works for you @ehamberg?

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 config.el file:

(use-package! auto-dark
  :config
  (setq custom-safe-themes t)
  (setq! auto-dark-themes '((doom-bluloco-dark) (doom-bluloco-light)))
  (auto-dark-mode))

ehamberg avatar Sep 20 '24 13:09 ehamberg

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?

LionyxML avatar Sep 21 '24 03:09 LionyxML

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 custom-safe-themes is only set to t temporarily when auto-dark--enable-themes is called. Here the use-package declaration that I use:

(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 auto-dark--enable-themes using describe-function; and ii) it restores custom-safe-themes automatically, assuming lexical binding is enabled.

Ohhh dang, I missed this message! Thanks for reaching @mepcotterell.

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 ?

LionyxML avatar Sep 21 '24 03:09 LionyxML

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, custom-safe-themes is going to be needed on all versions of Emacs.

But I think if you don’t just want to set it to t, the way to go about it is to limit it to the themes you intend to use, rather than toggling it on with advice for auto-dark. E.g.,

(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 defcustom auto-dark-assume-themes-are-safe, which would default to nil, but when set to t would have the same behavior as before #57. The effect would be like @mepcotterell’s advice, but simpler for users. Not as safe as listing explicit theme SHAs, but safer than setting custom-safe-themes to t, and better than the old behavior because the user still has to opt-in.

sellout avatar Sep 21 '24 06:09 sellout

Ok, I’ve thought about this more than I should.

First, I was wrong to suggest auto-dark-assume-themes-are-safe. The current mechanism for themes is pretty good. When you load (a particular version of) a theme the first time, Emacs will ask you “This can run Lisp code, are you sure?” and if you say yes, it’ll ask “Remeber that this theme is safe?” And a yes there will update your customizations to include the hash, so a user never has to do anything other than approve a theme or an update to a theme.

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 background-mode switches. But after answering the questions once, Emacs won’t complain any more (until you modify the themes).

@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 leuven would load, but everything would work properly after the first mode switch. It turned out I was accidentally enabling Auto Dark before my themes were set, so it loaded the Auto Dark defaults, but on future switches, had my expected themes. Since (auto-dark-mode) only causes a switch if the mode has changed, is it possible you still have something like the snippet from https://github.com/LionyxML/auto-dark-emacs/issues/64#issuecomment-2362309913 earlier in your config, causing Auto Dark to load doom-one, then the vars are updated, but the second call to auto-dark-mode does nothing, since the mode hasn’t changed?


I’ll adjust this based on responses, but here’s what I’m thinking now:

  1. Update the README/docstrings explaining custom-safe-themes with the recommendation to answer yes/yes to get themes explicitly added to the list rather than using (setq custom-safe-themes t).
  2. Update the README calling out the potential bad behavior of Doom Emacs failing during initialization (would love more info about this)
  3. Modify auto-dark--check-and-set-dark-mode to set the themes if the current variables don’t match custom-enabled-themes (rather than only if light/dark mode has changed)
  4. Modify auto-dark-themes to
    • call (load-theme _ nil t) on all themes, which should reduce the occurrence of being prompted about safe themes at initialization or mode switch and
    • call auto-dark--check-and-set-dark-mode after it’s set, so that (with point 3 above) the current themes update immediately

Thoughts?

sellout avatar Sep 23 '24 21:09 sellout

So many people helping! 😍 Nice!

@ehamberg, for me, it works with both after doom-themes and the way you provided. But you know, I only installed Doom for testing, with a very basic setup—just LSP, Tree-sitter, completions, and nothing else. Maybe you have some other settings in early-init.el, custom-vars.el, or even conflicting configurations in ~/.emacs.d or ~/.emacs with ~/.config/emacs? Just taking a guess. 😄

@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 load-theme commands or themes manually selected via M-x customize-themes, or even something setting a dark theme in early-init.el to prevent bright flashes when opening GUI Emacs. These can all potentially interfere with auto-dark's behavior.

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.

LionyxML avatar Sep 24 '24 22:09 LionyxML

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 avatar Sep 24 '24 22:09 sellout

@sellout I was getting the confirmation prompt every time I started Emacs, despite the proper hashes being present in custom-safe-themes, because my custom-file is loaded at the end of my init file (after auto-dark is enabled).

If I defer calling auto-dark-mode until after my custom-file is loaded, that fixes the prompt issue but results in the default theme being displayed for longer than I would like.

I know that I can set custom-safe-themes in the use-package call for auto-dark, but I would prefer not to do that, because I only load auto-dark when (display-graphics-p) is non-nil and use a different package to switch between the same themes when running Emacs in the terminal.

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 auto-dark-mode is enabled, then I feel like I have bigger issues. I did consider adjusting my advice to hash a theme the first time (or each time) it’s loaded by auto-dark and issue a warning in an after-init-hook if the hash is not present in custom-safe-themes, but that still lets an unsafe theme get loaded. I suppose I could try to manually read the custom-file to do that check sooner, but that makes assumptions about when it’s set, and in a situation where malicious code is already modifying the load path, the value of custom-file can’t really be trusted either.

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 custom-safe-themes, and lets my custom-file continue to be loaded at the end of my init.

mepcotterell avatar Sep 25 '24 05:09 mepcotterell

Ah, thanks @mepcotterell, that context really helps.

If other elisp code maliciously adjusts the load path so that a different theme with the same name is loaded when auto-dark-mode is enabled, then I feel like I have bigger issues.

Absolutely – custom-safe-themes is just meant to ensure that you’ve vetted new theme code … but there are so many loopholes for malicious code in Emacs that it seems rather performative (Emacs doesn’t complain at all when I load an arbitrary Lisp file, which can do whatever it wants to my system … including the dreaded (load-theme _ t)).

I like to load my custom-file early, but I don’t have much in there. Most of my customization is done via use-package, and most custom-file settings (except for things like custom-safe-themes, which are too finicky) migrate to use-package as I decide to persist them (since my custom-file is machine-local, but my init with use-package is shared across all my accounts).

In any case, I’ll join you in thinking about what might be a good solution for this case.

sellout avatar Sep 25 '24 06:09 sellout

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.

sellout avatar Sep 25 '24 08:09 sellout

Ok, I’ve thought about this more than I should.

😬

@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).

Yeah, that could definitely be.

@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?

I get the prompt, but nothing but the bottom bar (“modline”?) updates:

https://github.com/user-attachments/assets/b12ad4dd-3df8-418e-b481-40faf4f0b6ac

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.

It does not prompt, and stuff (modulo auto dark, naturally) works.

@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 leuven would load, but everything would work properly after the first mode switch. It turned out I was accidentally enabling Auto Dark before my themes were set, so it loaded the Auto Dark defaults, but on future switches, had my expected themes. Since (auto-dark-mode) only causes a switch if the mode has changed, is it possible you still have something like the snippet from #64 (comment) earlier in your config, causing Auto Dark to load doom-one, then the vars are updated, but the second call to auto-dark-mode does nothing, since the mode hasn’t changed?

Even if I only have this in my 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 '((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).

ehamberg avatar Sep 25 '24 10:09 ehamberg

@sellout Thanks for all your work on this. I really appreciate being able to have a conversation about it.

I like to load my custom-file early, but I don’t have much in there.

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 (custom-set-variables ...) and (custom-set-faces ...) by default. It's also worth considering the expected behavior and related user experience of the "Treat this theme as safe in future sessions?" prompt. if a user encounters this prompt during initialization and answers with y, then ((customize-push-and-save 'custom-safe-themes (list hash)) is called and the actual modification of custom-safe-themes is deferred using an after-init-hook. If loading customizations late is typical, then this results in auto-dark's call to load-theme relying on custom-safe-themes before it's set for most users.

Most of my customization is done via use-package, and most custom-file settings (except for things like custom-safe-themes, which are too finicky) migrate to use-package as I decide to persist them (since my custom-file is machine-local, but my init with use-package is shared across all my accounts).

I typically migrate package-specific custom variables into a package's use-package call as well, but custom-safe-themes isn't defined by auto-dark and, at least in my case, and is needed by more than package. Also, setting custom-safe-themes in the use-package call can lead to confusion when users reply with y to the "Treat this theme as safe in future sessions?" prompt since they would still need to manually migrate that change into their use-package call before it maybe has the intended effect, depending on when and where their customizations are loaded.

Finally, since load-theme inlines the hash calculation instead of factoring it out into a separate function, looking at wherever customize-push-and-save puts the hash is the only convenient way for most users to even get the hash in the first place. That makes it harder or more confusing for users who may want to set custom-safe-themes in their use-package call from the get go.

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.

mepcotterell avatar Sep 25 '24 11:09 mepcotterell

@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.

mepcotterell avatar Sep 25 '24 11:09 mepcotterell

I really appreciate being able to have a conversation about it.

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 custom-safe-themes without the user’s sign-off. I.e., it shouldn’t set it or call (load-themes _ t). And of course, my opinion there combined with my previous PR is what has led to this issue (and, to some extent, my high level of engagement here). But again, if @LionyxML says “put (load-theme _ t) back in,” I will and that’s the end of it.

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 user-init-file by default, since one of the benefits of Customize over setq, etc. is that it’s declarative. I.e., you can do all of your customization up-front, before the packages are loaded, and then you can be sure that those variables have the correct values no matter where they’re referenced.

In the spirit of @mpgcottel’s list of things they want to achieve with the config, I want

  1. Auto-Dark to not suppress the prompts without user sign-off,
  2. users to not get inundated with prompts (or to have initialization fail because of prompts!), and
  3. the configuration of Auto-Dark to be minimal and obvious (users shouldn’t have to read a manual to get it to behave correctly).

The auto-dark-assume-safe-themes variable I proposed earlier would seem to address the first two points (at the cost of the 3rd), but really doesn’t as the vanilla Emacs setup (putting custom-set-variables at the end of the init file) results in the same issue as my fourth example below.

Some different approaches to configuration:

  1. no prompt Doesn’t trigger a prompt. This is unusual (I think?) but should make things more declarative overall (I’ve often wished that use-package built up a single function to evaluate in after-init-hook to make it more predictable).

    (add-hook 'after-init-hook 'auto-dark-mode)
    
    (custom-set-variables
      '(custom-safe-themes t))
    
  2. no prompt This is the style I use (at least for the things that don’t move to use-package), and I feel like it’s the most natural – declare variable values, then perform various actions that use them. But as @mepcotterell points out, it’s not how Emacs does things by default.

    (custom-set-variables
      '(custom-safe-themes t))
    
    (auto-dark-mode)
    
  3. no prompt use-package handles custom “properly”, setting the variables before :init, which again is different from Emacs’ default. And I don’t know if it’s bad form to set custom-safe-themes here, since it’s not part of this package. I wouldn’t do it this way, but I probably would write up a README this way to assist newer users.

    (use-package auto-dark
      :custom (custom-safe-themes t)
      :init (auto-dark-mode))
    
  4. prompt This is the vanilla Emacs way, but it behaves incorrectly. As @mepcotterell points out, if your only config is (auto-dark-mode) and you answer the prompts the first time through, this is the config you’ll end up with, and Emacs will continue to prompt you every time![^1]

    (auto-dark-mode)
    
    (custom-set-variables
      '(custom-safe-themes t))
    
  5. prompt While use-package claims to support order-independence, setting up dependencies like this doesn’t actually do things in the order you want (since custom is already loaded, auto-dark will load before use-package custom is read).

    (use-package auto-dark
      :after custom
      :init (auto-dark-mode))
    
    (use-package custom
      :custom (custom-safe-themes t))
    

[^1]: My assumption is that Emacs puts custom-set-variables at the end to try to hide the noise (and code you shouldn’t edit manually) rather than putting it front-and-center at the top. But I don’t know why Emacs didn’t just add custom-file as another initialization step before loading user-init-file, which would benefit its declarative nature, and make it easier for users to not accidentally edit it manually.

I feel like Emacs wants users to let it manage the set of custom-safe-theme hashes for them, but I think it also encourages a configuration style that prevents that value from being available to Auto-Dark, as @mpgcottel is encountering.

So, I don’t yet see a solution that satisfies my criteria[^2] (which as I mentioned are of secondary importance on this repo).

[^2]: While doing this, I have been taking more general notes and going through the initialization process a bit more thoroughly than the manual does. I’m hoping to publish something helpful from that, as well as a more opinionated recommendation of how to set up ones on configuration … which this discussion is helping me understand.

since load-theme inlines the hash calculation instead of factoring it out into a separate function, looking at wherever customize-push-and-save puts the hash is the only convenient way for most users to even get the hash in the first place.

Yeah, this is quite annoying.

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.

Yes – I think this is largely out of the scope of this project, aside from the README/docs informing the user of custom-set-themes and how to use it (since the prompts from Emacs don’t tell you how to silence those prompts more permanently).

I.e., I feel like if Auto-Dark has a variable like auto-dark-assume-safe-themes that the user has to discover and set, then we can help them discover custom-safe-themes just as easily.

sellout avatar Sep 26 '24 15:09 sellout

@ehamberg And I haven’t forgotten about your issues – just need to dig in more about what’s going on there.

sellout avatar Sep 26 '24 15:09 sellout

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:

  1. Auto-Dark to not suppress the prompts without user sign-off,
  2. users to not get inundated with prompts (or to have initialization fail because of prompts!), and
  3. the configuration of Auto-Dark to be minimal and obvious (users shouldn’t have to read a manual to get it to behave correctly).

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 auto-dark-assume-safe-themes. Without thinking too much, it seems a really good idea, maybe a name that the user will think a bit before setting with 'UNSAFE' or 'bypass-safety-check' or something like it would be better.

LionyxML avatar Sep 27 '24 13:09 LionyxML

Hello everyone!

Not sure why I kept this issue open 😅

Anyway, I just tagged a new release of auto-dark - v0.13.14 and it should propagate to the package archives shortly.

If anyone using Doom Emacs could confirm that everything is working as expected, I’ll go ahead and close this issue.

Thanks! 🙏

LionyxML avatar Jul 21 '25 22:07 LionyxML

(Using 0aa6df7f561e702fd42f2436544b957434e93ad9 on the latest Doom Emacs)

If I change to the config shown in the current README:

(after! doom-ui
  ;; set your favorite themes
  (setq! auto-dark-themes '((doom-bluloco-dark) (doom-bluloco-light)))
  (auto-dark-mode))

…I get the prompts on startup again, and initialization is broken. 😬

After reverting back to my old config, things work again:

(use-package! auto-dark
  :config
  (setq custom-safe-themes t)
  (setq! auto-dark-themes '((doom-bluloco-dark) (doom-bluloco-light)))

  ;; Add hooks to update doom-theme variable
  (add-hook! 'auto-dark-dark-mode-hook (setq doom-theme 'doom-bluloco-dark))
  (add-hook! 'auto-dark-light-mode-hook (setq doom-theme 'doom-bluloco-light))

  (when (display-graphic-p)
    (auto-dark-mode)))

ehamberg avatar Aug 03 '25 10:08 ehamberg

@ehamberg

With https://github.com/doomemacs/doomemacs/issues/8119 resolved, doom doesn't use doom-theme any longer to track the active theme, so you won't need the hocks.

But I still need (setq custom-safe-themes t) in my config, yes.

This is my current config, battle-tested over months (changed to the new auto-dark-themes):

(use-package! auto-dark
  :defer t
  :init
  ;; Configure themes
  (setq! auto-dark-themes '((doom-solarized-dark) (doom-solarized-light)))
  ;; Disable doom's theme loading mechanism
  (setq! doom-theme nil)
  ;; All themes are safe. (If there is malicious code in an installed package,
  ;; it's game over anyway.)
  (setq! custom-safe-themes t)
  ;; Enable auto-dark-mode at the right point in time.
  ;; This is inspired by doom-ui.el. Using server-after-make-frame-hook avoids
  ;; issues with an early start of the emacs daemon using systemd, which causes
  ;; problems with the DBus connection that auto-dark mode relies upon.
  (defun >auto-dark-init-h ()
    (auto-dark-mode)
    (remove-hook 'server-after-make-frame-hook #'>auto-dark-init-h)
    (remove-hook 'after-init-hook #'>auto-dark-init-h))
  (let ((hook (if (daemonp)
                  'server-after-make-frame-hook
                'after-init-hook)))
    ;; Depth -95 puts this before doom-init-theme-h, which sounds like a good
    ;; idea, if only for performance reasons.
    (add-hook hook #'>auto-dark-init-h -95)))

This also got a few thumbs-up in https://github.com/doomemacs/doomemacs/issues/6424#issuecomment-2347930867. If this works for you too, I suggest putting it in the README here... Doom remains a bit complicated, but this is how things are currently.

real-or-random avatar Aug 06 '25 03:08 real-or-random

@real-or-random Thanks! That seem to work well. (But, oh my!, that's a lot of moving parts. 😬 )

ehamberg avatar Aug 06 '25 08:08 ehamberg

Here's some more rationale.

But, oh my!, that's a lot of moving parts. 😬

True, but it's more than it looks on first glance.

  • (setq! doom-theme nil) is just to make sure that no time is spent on loading two themes. The default of the variable is nil but it's set to doom-one in the example config. I don't think it's required at all, but it also won't hurt. (If I set this to an invalid theme, I don't get any error message that doom tried to load the invalid theme, so auto-dark-mode kicks in early enough.)
  • And the code that selects one of two hooks depending on whether Emacs is run in daemon to fix DBus issues may not be specific to Doom at all. I don't know, I haven't tested elsewhere but the hooks are built-in Emacs hooks, so this could be used everywhere or at least everywhere on Linux. What this does is to delay initializing auto-dark-mode as much as possible, i.e., until the first frame is opened. At this time one should hopefully have a working DBus server to connect to. The thing is just that there's no generic after-make-frame-hook, so if we're not in daemon mode, then server-after-make-frame-hook is not fired at all, and we need to fall back to after-init-hook (which is fine because then Emacs was probably started manually at a time when the DBus server is already present.)

real-or-random avatar Aug 06 '25 11:08 real-or-random

@real-or-random , hello there o/

This is my current config, battle-tested over months (changed to the new auto-dark-themes):

Would you mind if I add it to the README? If you'd prefer to open a PR yourself, I’d be more than happy to merge it.

LionyxML avatar Aug 06 '25 12:08 LionyxML

See #88.

--

For the rest of this issue:

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 auto-dark-mode is enabled, then I feel like I have bigger issues.

I tend to agree with this, as long as it's properly documented. We could make this explicit in the installation docs: "List only themes here that you trust."

real-or-random avatar Aug 06 '25 15:08 real-or-random