ef-themes-select-{dark,light} triggers wrong-type-argument on completion

[league]

Hi Prot, I encountered this issue. I'm using current git-main (62723ea). This should reproduce it from scratch, I have emacs-28.2 and will run it within the ef-themes work-tree:

shell:  emacs -Q
eval:   (add-to-list 'load-path ".")
eval:   (require 'ef-themes)
keys:   M-x toggle-debug-on-error RET
keys:   M-x ef-themes-select-light RET
prompt: Select light Ef theme:
keys:   TAB
prompt: Select light Ef theme: ef-
keys:   TAB
error:  (Wrong type argument: stringp nil)

Backtrace is below, but what seems to be happening is that it's trying to put together the annotated theme list for completion purposes, but the 'theme-documentation property is missing from these symbols, so split-string gets nil as its first arg.

  split-string(nil "\\.")
  (car (split-string (get (intern theme) 'theme-documentation) "\\."))
  (format " -- %s" (car (split-string (get (intern theme) 'theme-documentation) "\\.")))
  ef-themes--annotate-theme(#("ef-cyprus" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))))
  #f(compiled-function (s) #<bytecode 0xfb00fbf0dc3aa01>)(#("ef-cyprus" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))))
  mapcar(#f(compiled-function (s) #<bytecode 0xfb00fbf0dc3aa01>) (#("ef-cyprus" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-day" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-deuteranopia-light" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-duo-light" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-frost" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-kassio" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-light" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-spring" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-summer" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-trio-light" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference))) #("ef-tritanopia-light" 0 3 (face (completions-common-part)) 3 4 (face (completions-first-difference)))))
  #f(compiled-function (window) #<bytecode -0x1d915406e920c6dd>)(#<window 14 on *Completions*>)
  window--display-buffer(#<buffer *Completions*> #<window 14 on *Completions*> window ((window-height . fit-window-to-buffer) nil (body-function . #f(compiled-function (window) #<bytecode -0x1d915406e920c6dd>))))
  display-buffer-at-bottom(#<buffer *Completions*> ((window-height . fit-window-to-buffer) nil (body-function . #f(compiled-function (window) #<bytecode -0x1d915406e920c6dd>))))
  display-buffer(#<buffer *Completions*> ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame display-buffer-at-bottom) (window-height . fit-window-to-buffer) nil (body-function . #f(compiled-function (window) #<bytecode -0x1d915406e920c6dd>))))
  temp-buffer-window-show(#<buffer *Completions*> ((display-buffer--maybe-same-window display-buffer-reuse-window display-buffer--maybe-pop-up-frame display-buffer-at-bottom) (window-height . fit-window-to-buffer) nil (body-function . #f(compiled-function (window) #<bytecode -0x1d915406e920c6dd>))))
  minibuffer-completion-help(24 27)
  completion--do-completion(24 27)
  completion--in-region-1(24 27)
  #f(compiled-function (start end collection predicate) #<bytecode 0x1693eb75ae165d41>)(24 27 (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil)
  apply(#f(compiled-function (start end collection predicate) #<bytecode 0x1693eb75ae165d41>) (24 27 (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil))
  #f(compiled-function (funs global args) #<bytecode -0x73c10d0b2c91f62>)(nil nil (24 27 (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil))
  completion--in-region(24 27 (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil)
  completion-in-region(24 27 (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil)
  minibuffer-complete()
  funcall-interactively(minibuffer-complete)
  command-execute(minibuffer-complete)
  completing-read-default("Select light Ef theme: " (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil t nil ef-themes--select-theme-history nil nil)
  completing-read("Select light Ef theme: " (ef-cyprus ef-day ef-deuteranopia-light ef-duo-light ef-frost ef-kassio ef-light ef-spring ef-summer ef-trio-light ef-tritanopia-light) nil t nil ef-themes--select-theme-history)
  (intern (completing-read (or prompt "Select Ef Theme: ") themes nil t nil 'ef-themes--select-theme-history))
  (let* ((subset (ef-themes--maybe-prompt-subset variant)) (themes (ef-themes--load-subset subset)) (completion-extra-properties (list ':annotation-function #'ef-themes--annotate-theme))) (intern (completing-read (or prompt "Select Ef Theme: ") themes nil t nil 'ef-themes--select-theme-history)))
  ef-themes--select-prompt("Select light Ef theme: " light)
  (list (ef-themes--select-prompt "Select light Ef theme: " 'light))
  command-execute(ef-themes-select-light record)
  execute-extended-command(nil "ef-themes-select-light" "ef-themes-select-light")
  funcall-interactively(execute-extended-command nil "ef-themes-select-light" "ef-themes-select-light")
  command-execute(execute-extended-command)

A few more notes:

• Same thing happens with ef-themes-select-dark, but not with ef-themes-select.
• The theme symbols don't have any properties at this point, so I guess it's a loading or load-ordering issue.
• There is enough of a pause in ef-themes-select before it presents the prompt, that I imagine something is triggering loading all the theme files and property lists. But it's not happening for the select-dark and select-light variants.
• Before I reduced this to reproduce with emacs -Q, I had been loading a default theme from my init using (ef-themes-select 'ef-frost). In this case, ef-frost has its property list including theme-documentation, but other theme symbols still don't.
• Aside: the prompt and doc string for ef-themes-select-dark still says "Light" in 2 places. I was about to send a patch just for that typo when I noticed the Lisp error on completion.

Thanks!

[league]

Ah, in this function:

(defun ef-themes--load-subset (subset)
  "Return the `light' or `dark' SUBSET of the Ef themes.
If SUBSET is neither `light' nor `dark', return all the known Ef themes."
  (pcase subset
    ('dark ef-themes-dark-themes)
    ('light ef-themes-light-themes)
    (_ (ef-themes--list-known-themes))))

In the default case (no subset specified), ef-themes--list-known-themes takes care to call ef-themes--enable-themes. But in the light/dark cases, it's just a list of symbols that won't correspond to loaded themes (unless they were already loaded through another invocation).

[protesilaos]

From: Christopher League ***@***.***> Date: Thu, 8 Jun 2023 05:16:15 -0700 Ah, in this function: ```elisp (defun ef-themes--load-subset (subset) "Return the `light' or `dark' SUBSET of the Ef themes. If SUBSET is neither `light' nor `dark', return all the known Ef themes." (pcase subset ('dark ef-themes-dark-themes) ('light ef-themes-light-themes) (_ (ef-themes--list-known-themes)))) ``` In the default case (no subset specified), `ef-themes--list-known-themes` takes care to call `ef-themes--enable-themes`. But in the light/dark cases, it's just a list of symbols that won't correspond to loaded themes (unless they were already loaded through another invocation).

You are right! We need to enable the themes in all cases. Do you want to submit a patch?

…

-- Protesilaos Stavrou https://protesilaos.com

[league]

Sure, I can send a patch later today – assuming that's the direction to go (it seems simplest). I was thinking another approach might be to reduce the completion information presented whenever theme-documentation is unavailable (graceful degradation). But my guess is that the uniformity of “load everything ahead of time” is more appealing.

[protesilaos]

From: Christopher League ***@***.***> Date: Thu, 8 Jun 2023 05:51:15 -0700 Sure, I can send a patch later today – assuming that's the direction to go (it seems simplest). I was thinking another approach might be to reduce the completion information presented whenever theme-documentation is unavailable (graceful degradation). But my guess is that the uniformity of “load everything ahead of time” is more appealing.

I like the approach of graceful degradation! I had the "enable everything" approach as a discovery mechanism. Though that may be too much for existing users who already know the themes they want.

…

-- Protesilaos Stavrou https://protesilaos.com