[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#52003: Unexpected advising behavior due to recursive implementation
From: |
Daniel Sausner |
Subject: |
bug#52003: Unexpected advising behavior due to recursive implementation |
Date: |
Sat, 20 Nov 2021 16:13:32 +0000 |
Hi,
I stumbled [1] on an issue that seems to affect several functions [2] in
lisp/emacs-lisp/lisp.el. For the sake of brevity I'll sketch it only for
forward-sexp but the problematic code is effectively duplicated and was
introduced with a commit [3] about one year ago.
Here's the problem: Since the commit any advice on the function
forward-sexp will effectively be called twice before the actual code
runs in interactive mode. In non-interactive mode everthing is as
expected however.
The reason is the introduction of an error message if no
forward/backward sexp is found. This is implemented in a way that the
functions calls itself immediately again and scans for errors:
(defun forward-sexp (&optional arg interactive)
"..."
(interactive "^p\nd")
(if interactive
(condition-case _
(forward-sexp arg nil) <-- Recursion
(scan-error (user-error (if (> arg 0)
"No next sexp"
"No previous sexp"))))
(or arg (setq arg 1))
(if forward-sexp-function
(funcall forward-sexp-function arg)
(goto-char (or (scan-sexps (point) arg) (buffer-end arg)))
(if (< arg 0) (backward-prefix-chars)))))
In my (very) humble opinion that method of error catching was an
unfortunate choice in that regard, that it makes the advising very
counter-intuitive.
I'm far from a lisp expert but my feeling is that the condition-case
should only wrap the calls where things can actually go wrong.
If there is interest, I'd be happy to provide a patch :-)
Best regards,
Daniel
[1] https://github.com/emacs-evil/evil/issues/1541
[2] On a first glimpse at least: forward-sexp, forward-list, down-list,
kill-sexp in that particular file.
[3] Commit:
df0f32f04850e7ed106a225addfa82f1b5b91f45
Author: Mattias Engdegård <mattiase@acm.org>
AuthorDate: Fri Sep 18 12:49:33 2020 +0200
Commit: Mattias Engdegård <mattiase@acm.org>
CommitDate: Wed Sep 23 16:31:18 2020 +0200
Don't signal scan-error when moving by sexp interactively
- bug#52003: Unexpected advising behavior due to recursive implementation,
Daniel Sausner <=