GNU bug report logs - #30697
make eval-expression only take `read'-able Lisp

Previous Next

Package: emacs;

Reported by: charles <at> aurox.ch (Charles A. Roelli)

Date: Sun, 4 Mar 2018 16:36:01 UTC

Severity: wishlist

Tags: fixed, patch

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 30697 in the body.
You can then email your comments to 30697 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Sun, 04 Mar 2018 16:36:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to charles <at> aurox.ch (Charles A. Roelli):
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 04 Mar 2018 16:36:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: bug-gnu-emacs <at> gnu.org
Subject: make eval-expression only take `read'-able Lisp
Date: Sun, 04 Mar 2018 17:46:18 +0100
Here is a change to make eval-expression issue a warning in the
minibuffer when the user enters an unreadable expression.  Currently,
we just error out, so the user has to restart the command and go back
in the history to get back what he typed.

With the change applied, if you type

M-: ( RET

from emacs -q, you'll see,

Eval: ( [End of file during parsing]

which gives you a chance to fix the error right away.

As another example, if you type an extra paren, as in

M-: ( ) ) RET

you'll see,

Eval: ()) [Trailing garbage following expression]

and point is left at the extra paren.


I've also added documentation for read--expression.  The change
follows:

diff --git a/lisp/simple.el b/lisp/simple.el
index 60a0028..7387554 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1517,6 +1517,10 @@ eval-expression-minibuffer-setup-hook
   "Hook run by `eval-expression' when entering the minibuffer.")
 
 (defun read--expression (prompt &optional initial-contents)
+  "Read an Emacs Lisp expression from the minibuffer.
+
+PROMPT and optional argument INITIAL-CONTENTS do the same as in
+function `read-from-minibuffer'."
   (let ((minibuffer-completing-symbol t))
     (minibuffer-with-setup-hook
         (lambda ()
@@ -1526,11 +1530,52 @@ read--expression
           (eldoc-mode 1)
           (add-hook 'completion-at-point-functions
                     #'elisp-completion-at-point nil t)
+          (local-set-key "\r" 'read--expression-try-read)
+          (local-set-key "\n" 'read--expression-try-read)
           (run-hooks 'eval-expression-minibuffer-setup-hook))
       (read-from-minibuffer prompt initial-contents
                             read-expression-map t
                             'read-expression-history))))
 
+(defun read--expression-try-read ()
+  "Try to read an Emacs Lisp expression in the minibuffer.
+
+Exit the minibuffer if successful, else report the error to the
+user and move point to the location of the error.  If point is
+not already at the location of the error, push a mark before
+moving point."
+  (interactive)
+  (unless (> (minibuffer-depth) 0)
+    (error "Minibuffer must be active"))
+  (if (let* ((contents (minibuffer-contents))
+             (error-point nil))
+        (with-temp-buffer
+          (condition-case err
+              (progn
+                (insert contents)
+                (goto-char (point-min))
+                ;; `read' will signal errors like "End of file during
+                ;; parsing" and "Invalid read syntax".
+                (read (current-buffer))
+                ;; Since `read' does not signal the "Trailing garbage
+                ;; following expression" error, we check for trailing
+                ;; garbage ourselves.
+                (or (progn
+                      ;; This check is similar to what `string_to_object'
+                      ;; does in minibuf.c.
+                      (skip-chars-forward " \t\n")
+                      (= (point) (point-max)))
+                    (error "Trailing garbage following expression")))
+            (error
+             (setq error-point (+ (length (minibuffer-prompt)) (point)))
+             (with-current-buffer (window-buffer (minibuffer-window))
+               (unless (= (point) error-point)
+                 (push-mark))
+               (goto-char error-point)
+               (minibuffer-message (error-message-string err)))
+             nil))))
+      (exit-minibuffer)))
+
 (defun eval-expression-get-print-arguments (prefix-argument)
   "Get arguments for commands that print an expression result.
 Returns a list (INSERT-VALUE NO-TRUNCATE CHAR-PRINT-LIMIT)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Sun, 04 Mar 2018 17:12:02 GMT) Full text and rfc822 format available.

Message #8 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 30697 <at> debbugs.gnu.org
Subject: Re: bug#30697: make eval-expression only take `read'-able Lisp
Date: Sun, 04 Mar 2018 19:10:55 +0200
> Date: Sun, 04 Mar 2018 17:46:18 +0100
> From: charles <at> aurox.ch (Charles A. Roelli)
> 
> Here is a change to make eval-expression issue a warning in the
> minibuffer when the user enters an unreadable expression.  Currently,
> we just error out, so the user has to restart the command and go back
> in the history to get back what he typed.
> 
> With the change applied, if you type
> 
> M-: ( RET
> 
> from emacs -q, you'll see,
> 
> Eval: ( [End of file during parsing]
> 
> which gives you a chance to fix the error right away.
> 
> As another example, if you type an extra paren, as in
> 
> M-: ( ) ) RET
> 
> you'll see,
> 
> Eval: ()) [Trailing garbage following expression]
> 
> and point is left at the extra paren.

You are changing the behavior of read--expression, which has a few
callers in Emacs.  Did you verify that those callers won't break due
to this change?

This should also have a NEWS entry, and perhaps the manual needs some
changes, please take a look.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Sun, 04 Mar 2018 22:38:02 GMT) Full text and rfc822 format available.

Message #11 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: "Charles A. Roelli" <charles <at> aurox.ch>, 30697 <at> debbugs.gnu.org
Subject: RE: bug#30697: make eval-expression only take `read'-able Lisp
Date: Sun, 4 Mar 2018 14:36:57 -0800 (PST)
> Here is a change to make eval-expression issue a warning in the
> minibuffer when the user enters an unreadable expression.  Currently,
> we just error out, so the user has to restart the command and go back
> in the history to get back what he typed.

I don't see why this is better than what happens now.

We currently have `eval-expression-debug-on-error', which helps
you understand the read or eval error.  And the function is not
necessarily called interactively ("When called interactively...").

What problem is the proposed change trying to solve?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Mon, 05 Mar 2018 18:42:01 GMT) Full text and rfc822 format available.

Message #14 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 30697 <at> debbugs.gnu.org
Subject: Re: bug#30697: make eval-expression only take `read'-able Lisp
Date: Mon, 05 Mar 2018 19:52:14 +0100
> Date: Sun, 04 Mar 2018 19:10:55 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> CC: 30697 <at> debbugs.gnu.org
> 
> You are changing the behavior of read--expression, which has a few
> callers in Emacs.  Did you verify that those callers won't break due
> to this change?

I've now tested them, and they work fine.  `eval-minibuffer' (which is
used for (interactive "X")) turns out to be one of the callers of
`read--expression', so I think the documentation of that interactive
spec will have to be updated too.

I also did small tests with ido-mode/icomplete-mode enabled, which
also do not seem to be affected by this change.
 
> This should also have a NEWS entry, and perhaps the manual needs some
> changes, please take a look.

Thanks, I will work on that.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Mon, 05 Mar 2018 18:59:02 GMT) Full text and rfc822 format available.

Message #17 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 30697 <at> debbugs.gnu.org
Subject: Re: bug#30697: make eval-expression only take `read'-able Lisp
Date: Mon, 05 Mar 2018 20:09:19 +0100
> Date: Sun, 4 Mar 2018 14:36:57 -0800 (PST)
> From: Drew Adams <drew.adams <at> oracle.com>
> Content-Type: text/plain; charset=us-ascii
> 
> > Here is a change to make eval-expression issue a warning in the
> > minibuffer when the user enters an unreadable expression.  Currently,
> > we just error out, so the user has to restart the command and go back
> > in the history to get back what he typed.
> 
> I don't see why this is better than what happens now.
> 
> We currently have `eval-expression-debug-on-error', which helps
> you understand the read or eval error.  And the function is not
> necessarily called interactively ("When called interactively...").
> 
> What problem is the proposed change trying to solve?

It's too easy to evaluate a meaningless expression, which is never
intended.  Instead, we can point the user to the position of the error
so that it can be easily fixed, unlike the current behavior, where the
user is left guessing.

(I don't think eval-expression-debug-on-error catches read errors --
if it did, it would show a *Backtrace* buffer when an unreadable
expression is entered.  But I didn't look very deeply, so correct that
if it's wrong.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Mon, 24 Jun 2019 18:59:02 GMT) Full text and rfc822 format available.

Message #20 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 30697 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#30697: make eval-expression only take `read'-able Lisp,
 bug#30697: make eval-expression only take `read'-able Lisp
Date: Mon, 24 Jun 2019 20:58:23 +0200
charles <at> aurox.ch (Charles A. Roelli) writes:

> Here is a change to make eval-expression issue a warning in the
> minibuffer when the user enters an unreadable expression.  Currently,
> we just error out, so the user has to restart the command and go back
> in the history to get back what he typed.

I think this sounds like a good change -- I often type invalid stuff
into M-: and then have to M-: M-p, so this is a usability win.

>> You are changing the behavior of read--expression, which has a few
>> callers in Emacs.  Did you verify that those callers won't break due
>> to this change?
>
> I've now tested them, and they work fine.  `eval-minibuffer' (which is
> used for (interactive "X")) turns out to be one of the callers of
> `read--expression', so I think the documentation of that interactive
> spec will have to be updated too.
>
> I also did small tests with ido-mode/icomplete-mode enabled, which
> also do not seem to be affected by this change.
>
>> This should also have a NEWS entry, and perhaps the manual needs some
>> changes, please take a look.
>
> Thanks, I will work on that.

Did you finish up the change, Charles?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Mon, 10 Aug 2020 13:19:01 GMT) Full text and rfc822 format available.

Message #23 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 30697 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#30697: make eval-expression only take `read'-able Lisp,
 bug#30697: make eval-expression only take `read'-able Lisp
Date: Mon, 10 Aug 2020 15:17:52 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>>> This should also have a NEWS entry, and perhaps the manual needs some
>>> changes, please take a look.
>>
>> Thanks, I will work on that.
>
> Did you finish up the change, Charles?

This was a year ago, so I did added the NEWS entry.  I grepped through
the manuals for eval-expression to see whether the previous behaviour
was mentioned there (and would therefore need changing), but I couldn't
find anything, so I didn't change anything.

So this is now pushed to Emacs 28, and I think it's a really good and
natural change.  Thanks, Charles.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Added tag(s) fixed. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Mon, 10 Aug 2020 13:19:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.1, send any further explanations to 30697 <at> debbugs.gnu.org and charles <at> aurox.ch (Charles A. Roelli) Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Mon, 10 Aug 2020 13:19:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#30697; Package emacs. (Mon, 10 Aug 2020 13:21:01 GMT) Full text and rfc822 format available.

Message #30 received at 30697 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 30697 <at> debbugs.gnu.org
Subject: Re: bug#30697: make eval-expression only take `read'-able Lisp,
 bug#30697: make eval-expression only take `read'-able Lisp
Date: Mon, 10 Aug 2020 15:20:12 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> This was a year ago, so I did added the NEWS entry.  I grepped through
> the manuals for eval-expression to see whether the previous behaviour
> was mentioned there (and would therefore need changing), but I couldn't
> find anything, so I didn't change anything.

(I also ran a "make check", because there's a bunch of tests around this
area, and it didn't introduce any regressions.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 08 Sep 2020 11:24:13 GMT) Full text and rfc822 format available.

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

Previous Next


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