fix: Symbol’s function definition is void: window-purpose/save-dedicated-windows

[kassick]

Commit 1ce1b0ea21c2e1e68b0037329941c316b4dfef2e introduced a bug due to incorrect usage of define-advice.

This commit refactors the change by:

• Extracting the advices to named functions spacemacs/window-purpose-{save,restore}-dedicated-windows
• Using advice-add globally instead of a define-advice/advice-add mix

[kassick]

eb36a82 fixes an error that appeared once the spacemacs-purpose-popwin package was reinstalled (from layers/+spacemacs/spacemacs-purpose/local/spacemacs-purpose-popwin to ~/.emacs.d/elpa/${EMACS_VERSION}/develop/spacemacs-purpose-popwin-${BUILD_DATE}

If the error persist after integrating these commits, remove the spacemacs-purpose-popwin package from the elpa folder and restart emacs.

[kassick]

@heartnheart and @cormacc you may want to review your thumbs-up, since there are new commits.

[sunlin7]

Hi @kassick Thanks for the contributions. I didn't met an issue for

Commit https://github.com/syl20bnr/spacemacs/commit/1ce1b0ea21c2e1e68b0037329941c316b4dfef2e introduced a bug due to incorrect usage of define-advice.

Could you give more details to help us understanding what happened? Or, why it releated to define-advice Thanks

[kassick]

Hi @sunlin7 !

define-advice requires the parameter list for the function defined in body.

The followig snippet creates a function named hello@before-hello and adds it as a before advice to the target function:

(defun hello () (message "Hello World"))

(define-advice hello (:before (&rest args) before-hello)
  (message "before"))

The name is optional, though, so it can be used as

(define-advice hello (:before (&rest args))
  (message "before"))

to create an anonymous function.

Calling define-advice without the argument list does not work as expected, as the function created by define-advice is invalid. The following snippet yields Debugger entered--Lisp error: (invalid-function ((t) before-hello (message "before")))

(define-advice hello (:before before-hello)
  (message "before"))

(hello)

(Also, the function does not have a name, so anywhere expecting to use before-hello would fail with some symbol definition void error.)

I could have kept the use of define-advice by adding a (&rest args) to the definition, but then we'd need to adapt the code to point to name@advice-fn-name (e.g. popwin:create-popup-window@window-purpose/save-dedicated-windows) here and elsewhere in the code. I've preferred to use a simpler defun with the existing advice-add/advice-remove that is already present in the code.

---

As to why you did not encounter any issue, I'm not sure... Are you using the spacemacs-purpose layer? describe-function popwin:create-popup-window does show the advices if you are running spacemacs at 1ce1b0ea21c2e1e68b0037329941c316b4dfef2e ?

I'm using emacs 29.3.50 built from sources -- could it be due some version-specific behavior ?

[kassick]

Btw, the Symbol’s function definition is void error happens because using define-advice did not create a function named window-purpose/save-dedicated-windows (it ended up defining an invalid, anonymous function). But here, the layer adds window-purpose/save-dedicated-windows as an advice to popwin:create-popup-window -- it ends up before the invalid, anonymous function.

When popwin:create-popup-window is called, it first tries to run window-purpose/save-dedicated-windows, which does not exist -- ergo function definition is void. If this advice-add was not used, then the error would be invalid function.

[sunlin7]

:+1: Got the key point, thanks!