GNU bug report logs - #52003
Unexpected advising behavior due to recursive implementation

Previous Next

Package: emacs;

Reported by: Daniel Sausner <daniel.sausner <at> posteo.de>

Date: Sat, 20 Nov 2021 17:05:03 UTC

Severity: normal

Done: Mattias Engdegård <mattiase <at> acm.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Daniel Sausner <daniel.sausner <at> posteo.de>
Subject: bug#52003: closed (Re: bug#52003: Unexpected advising behavior
 due to recursive implementation)
Date: Mon, 22 Nov 2021 09:22:02 +0000
[Message part 1 (text/plain, inline)]
Your bug report

#52003: Unexpected advising behavior due to recursive implementation

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 52003 <at> debbugs.gnu.org.

-- 
52003: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=52003
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Mattias Engdegård <mattiase <at> acm.org>
To: Daniel Sausner <daniel.sausner <at> posteo.de>
Cc: 52003-done <at> debbugs.gnu.org
Subject: Re: bug#52003: Unexpected advising behavior due to recursive
 implementation
Date: Mon, 22 Nov 2021 10:20:57 +0100
21 nov. 2021 kl. 18.29 skrev Daniel Sausner <daniel.sausner <at> posteo.de>:

> The problem I'm trying to solve is, that the cursor in evil normal state is not between chars but _on_ a char. Moving to the end of a sexp in lisp I would expect the cursor to be on the closing paren instead of behind it.

> In essence I would like to move the visible cursor by a single char in one or the other direction before and after one or more `forward-sexp`-based commands are executed. But I'm not sure anymore if this is really worth the effort :-)

Thanks for the explanation. Sounds fiddly. Best of luck! I'm closing this bug then.


[Message part 3 (message/rfc822, inline)]
From: Daniel Sausner <daniel.sausner <at> posteo.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 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 <at> acm.org>
AuthorDate: Fri Sep 18 12:49:33 2020 +0200
Commit:     Mattias Engdegård <mattiase <at> acm.org>
CommitDate: Wed Sep 23 16:31:18 2020 +0200

Don't signal scan-error when moving by sexp interactively




This bug report was last modified 3 years and 180 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.