hydra--around-find-function-search-for-symbol-advice breaks xref

[mclearc]

With hydra--around-find-function-search-for-symbol-advice enabled none of the xref find definition functions work. I'm using emacs 28.2 on macOS 12.6 with the most recent version of hydra. I'm not sure what changed about emacs 28 to do this so I suspect it has to do with a commit in the last few months, perhaps one that backported to 28 from master.

EDIT: the error message is always related to a c file and says "wrong-type-argument listp" and then a path to the c file.

[basil-conto]

EDIT: the error message is always related to a c file and says "wrong-type-argument listp" and then a path to the c file.

Here's a repro:

1. $ cd "$(mktemp --directory)"
2. $ HOME="${PWD}" XDG_CONFIG_HOME="${PWD}/.config" emacs
3. M-: (setq package-archives '(("gnu" . "https://elpa.gnu.org/devel/"))) RET
4. M-x toggle-debug-on-error RET
5. M-x package-install RET ivy-hydra RET
6. M-x find-function RET when RET

Debugger entered--Lisp error:

  (wrong-type-argument listp "/home/blc/.local/share/emacs/30.0.50/lisp/subr.elc")
  apply(#f(compiled-function (symbol type library) "Search for SYMBOL's definition of type TYPE in LIBRARY.\nVisit the library in a buffer, and return a cons cell (BUFFER . POSITION),\nor just (BUFFER . nil) if the definition can't be found in the file.\n\nIf TYPE is nil, look for a function definition.\nOtherwise, TYPE specifies the kind of definition,\nand it is interpreted via `find-function-regexp-alist'.\nThe search is done in the source for library LIBRARY." #<bytecode -0x164e3e12ac289452>) when nil "/home/blc/.local/share/emacs/30.0.50/lisp/subr.elc")
  hydra--around-find-function-search-for-symbol-advice(#f(compiled-function (symbol type library) "Search for SYMBOL's definition of type TYPE in LIBRARY.\nVisit the library in a buffer, and return a cons cell (BUFFER . POSITION),\nor just (BUFFER . nil) if the definition can't be found in the file.\n\nIf TYPE is nil, look for a function definition.\nOtherwise, TYPE specifies the kind of definition,\nand it is interpreted via `find-function-regexp-alist'.\nThe search is done in the source for library LIBRARY." #<bytecode -0x164e3e12ac289452>) when nil "/home/blc/.local/share/emacs/30.0.50/lisp/subr.elc")
  apply(hydra--around-find-function-search-for-symbol-advice #f(compiled-function (symbol type library) "Search for SYMBOL's definition of type TYPE in LIBRARY.\nVisit the library in a buffer, and return a cons cell (BUFFER . POSITION),\nor just (BUFFER . nil) if the definition can't be found in the file.\n\nIf TYPE is nil, look for a function definition.\nOtherwise, TYPE specifies the kind of definition,\nand it is interpreted via `find-function-regexp-alist'.\nThe search is done in the source for library LIBRARY." #<bytecode -0x164e3e12ac289452>) (when nil "/home/blc/.local/share/emacs/30.0.50/lisp/subr.elc"))
  find-function-search-for-symbol(when nil "/home/blc/.local/share/emacs/30.0.50/lisp/subr.elc")
  find-function-noselect(when)
  find-definition-noselect(when nil)
  find-function-do-it(when nil switch-to-buffer)
  find-function(when)
  funcall-interactively(find-function when)
  call-interactively(find-function record nil)
  command-execute(find-function record)
  execute-extended-command(nil "find-function" "find-function")
  funcall-interactively(execute-extended-command nil "find-function" "find-function")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

With hydra--around-find-function-search-for-symbol-advice enabled none of the xref find definition functions work. I'm using emacs 28.2 on macOS 12.6 with the most recent version of hydra. I'm not sure what changed about emacs 28 to do this so I suspect it has to do with a commit in the last few months, perhaps one that backported to 28 from master.

The error does not come down to differences between Emacs versions, but rather differences in the version of hydra on GNU-devel ELPA versus that on GNU ELPA, MELPA Stable, or MELPA.

Namely, the version on GNU-devel ELPA is the only to contain the following change: https://git.sv.gnu.org/cgit/emacs/elpa.git/commit/?id=54e9db2b02

Note the use of apply instead of funcall in the function hydra--around-find-function-search-for-symbol-advice.

This is an unfortunate symptom of #408, i.e. that hydra.git and elpa.git have diverged due to (a) lack of maintenance in this repo, and (b) unilateral changes in elpa.git.

@monnier is the following fix for elpa.git okay? In particular, should I change the copyright years like this, as a continuous range, or leave them alone?

diff --git a/hydra.el b/hydra.el
index 5d5623c383..e6ee9f30e1 100644
--- a/hydra.el
+++ b/hydra.el
@@ -1,6 +1,6 @@
 ;;; hydra.el --- Make bindings that stick around. -*- lexical-binding: t -*-
 
-;; Copyright (C) 2015-2022  Free Software Foundation, Inc.
+;; Copyright (C) 2015-2022, 2024  Free Software Foundation, Inc.
 
 ;; Author: Oleh Krehel <[email protected]>
 ;; Maintainer: Oleh Krehel <[email protected]>
@@ -326,25 +326,28 @@ hydra-add-imenu
 
 ;; FIXME: Maybe we can dispense with this advice if `defhydra' adds appropriate
 ;; `definition-name' properties to the functions it defines?
+;; Note: Latest Emacs (at the time of writing, Emacs 30) is smart enough to find
+;; hydra definitions even without this advice or `definition-name'.
 (advice-add 'find-function-search-for-symbol :around
             #'hydra--around-find-function-search-for-symbol-advice)
 
 (defun hydra--around-find-function-search-for-symbol-advice
-    (orig-fun symbol type library)
+    (orig-fun symbol type library &rest args)
   "Navigate to hydras with `find-function-search-for-symbol'."
-  (let ((res (apply orig-fun symbol type library)))
-    (when (symbolp symbol)
-        ;; The original function returns (cons (current-buffer) (point))
-        ;; if it found the point.
-        (unless (cdr res)
-          (with-current-buffer (find-file-noselect library)
-            (let ((sn (symbol-name symbol)))
-              (when (and (null type)
-                         (string-match "\\`\\(hydra-[[:alnum:]-]+\\)/\\(.*\\)\\'" sn)
-                         (re-search-forward (concat "(defhydra " (match-string 1 sn))
-                                            nil t))
-                (goto-char (match-beginning 0)))
-              (cons (current-buffer) (point))))))
+  (let ((res (apply orig-fun symbol type library args)))
+    (when (and (symbolp symbol)
+               ;; Function definition.
+               (null type)
+               ;; Original function returns (BUFFER . POSITION)
+               ;; if it found POSITION; otherwise (BUFFER . nil).
+               (not (cdr res)))
+      (with-current-buffer (car res)
+        (let ((sn (symbol-name symbol)))
+          (save-excursion
+            (when (and (string-match "\\`\\(hydra-[[:alnum:]-]+\\)/.*\\'" sn)
+                       (search-forward (concat "(defhydra " (match-string 1 sn))
+                                       nil t))
+              (setq res (cons (car res) (match-beginning 0))))))))
     res))
 
 ;;* Universal Argument

Thanks.

[basil-conto]

In the meantime, the simplest workaround for anyone running into this error is to add the following to their user-init-file:

(with-eval-after-load 'hydra
  ;; See URL `https://github.com/abo-abo/hydra/issues/413'.
  (advice-remove 'find-function-search-for-symbol
                 'hydra--around-find-function-search-for-symbol-advice))

[monnier]

```diff diff --git a/hydra.el b/hydra.el index 5d5623c383..e6ee9f30e1 100644 --- a/hydra.el +++ b/hydra.el @@ -1,6 +1,6 @@ ;;; hydra.el --- Make bindings that stick around. -*- lexical-binding: t -*- -;; Copyright (C) 2015-2022 Free Software Foundation, Inc. +;; Copyright (C) 2015-2022, 2024 Free Software Foundation, Inc.

Since our changes include -;; Copyright (C) 2015-2019 Free Software Foundation, Inc. +;; Copyright (C) 2015-2022 Free Software Foundation, Inc. I'd just change the 2022 to 2024

@@ -326,25 +326,28 @@ hydra-add-imenu ;; FIXME: Maybe we can dispense with this advice if `defhydra' adds appropriate ;; `definition-name' properties to the functions it defines? +;; Note: Latest Emacs (at the time of writing, Emacs 30) is smart enough to find +;; hydra definitions even without this advice or `definition-name'. (advice-add 'find-function-search-for-symbol :around #'hydra--around-find-function-search-for-symbol-advice)

Interesting. Do you know which change in Emacs-30 gave us that? Could we (when (fboundp ???)) to skip the advice when it's not needed?

(defun hydra--around-find-function-search-for-symbol-advice - (orig-fun symbol type library) + (orig-fun symbol type library &rest args) "Navigate to hydras with `find-function-search-for-symbol'." - (let ((res (apply orig-fun symbol type library))) - (when (symbolp symbol) - ;; The original function returns (cons (current-buffer) (point)) - ;; if it found the point. - (unless (cdr res) - (with-current-buffer (find-file-noselect library) - (let ((sn (symbol-name symbol))) - (when (and (null type) - (string-match "\\`\\(hydra-[[:alnum:]-]+\\)/\\(.*\\)\\'" sn) - (re-search-forward (concat "(defhydra " (match-string 1 sn)) - nil t)) - (goto-char (match-beginning 0))) - (cons (current-buffer) (point)))))) + (let ((res (apply orig-fun symbol type library args))) + (when (and (symbolp symbol) + ;; Function definition. + (null type) + ;; Original function returns (BUFFER . POSITION) + ;; if it found POSITION; otherwise (BUFFER . nil). + (not (cdr res))) + (with-current-buffer (car res) + (let ((sn (symbol-name symbol))) + (save-excursion + (when (and (string-match "\\`\\(hydra-[[:alnum:]-]+\\)/.*\\'" sn) + (search-forward (concat "(defhydra " (match-string 1 sn)) + nil t)) + (setq res (cons (car res) (match-beginning 0)))))))) res))

Duh, yes, thanks. Tho maybe we should stick to fixing the missing `args`. BTW, if we want to move the tests into the main `when` like you did, we could also move the `string-match` test into it. Stefan