GNU bug report logs - #39291
M-: history doesn't store erroneous input

Previous Next

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

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

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

From: Paul Pogonyshev <pogonyshev <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: M-: history doesn't store erroneous input
Date: Sun, 26 Jan 2020 16:16:23 +0100

[Message part 1 (text/plain, inline)]

M-: (oops-i-forgot-to-type-the-closing-paren RET
=> End of file during parsing

Error itself is fine, but the problem is if you now
type M-: again, you won't find your input in the
history and have to retype everything from scratch
instead of just fixing the typo.

This is a regression in trunk compared at least to
26.3.

[Message part 2 (text/html, inline)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Paul Pogonyshev <pogonyshev <at> gmail.com>
Cc: 39291 <at> debbugs.gnu.org
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Wed, 29 Jan 2020 01:35:33 +0200

> M-: (oops-i-forgot-to-type-the-closing-paren RET
> => End of file during parsing
>
> Error itself is fine, but the problem is if you now
> type M-: again, you won't find your input in the
> history and have to retype everything from scratch
> instead of just fixing the typo.
>
> This is a regression in trunk compared at least to
> 26.3.

This was changed by
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9042ece787c
via https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38317#23
and https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00241.html

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>, Federico Tedin <federicotedin <at> gmail.com>
Cc: 39291 <at> debbugs.gnu.org, pogonyshev <at> gmail.com
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Wed, 29 Jan 2020 19:16:09 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Date: Wed, 29 Jan 2020 01:35:33 +0200
> Cc: 39291 <at> debbugs.gnu.org
> 
> > M-: (oops-i-forgot-to-type-the-closing-paren RET
> > => End of file during parsing
> >
> > Error itself is fine, but the problem is if you now
> > type M-: again, you won't find your input in the
> > history and have to retype everything from scratch
> > instead of just fixing the typo.
> >
> > This is a regression in trunk compared at least to
> > 26.3.
> 
> This was changed by
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9042ece787c
> via https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38317#23
> and https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00241.html

Federico, please look into fixing this regression.

TIA

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

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 39291 <at> debbugs.gnu.org, pogonyshev <at> gmail.com, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Wed, 29 Jan 2020 19:09:38 +0100

[Message part 1 (text/plain, inline)]

On Wed, 29 Jan 2020 at 18:16 Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Juri Linkov <juri <at> linkov.net>
> > Date: Wed, 29 Jan 2020 01:35:33 +0200
> > Cc: 39291 <at> debbugs.gnu.org
> >
> > > M-: (oops-i-forgot-to-type-the-closing-paren RET
> > > => End of file during parsing
> > >
> > > Error itself is fine, but the problem is if you now
> > > type M-: again, you won't find your input in the
> > > history and have to retype everything from scratch
> > > instead of just fixing the typo.
> > >
> > > This is a regression in trunk compared at least to
> > > 26.3.
> >
> > This was changed by
> > http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9042ece787c
> > via https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38317#23
> > and https://lists.gnu.org/archive/html/emacs-devel/2019-12/msg00241.html
>
> Federico, please look into fixing this regression.
>
> TIA


Thanks for letting me know, I will take a look at it.

>

[Message part 2 (text/html, inline)]

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

From: Federico Tedin <federicotedin <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 39291 <at> debbugs.gnu.org, pogonyshev <at> gmail.com, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Wed, 29 Jan 2020 22:32:16 +0100

[Message part 1 (text/plain, inline)]

> Federico, please look into fixing this regression.
>
> TIA

I found that the minibuffer input string in `read_minibuf' was being
added to the history list after we tried to parse an object from it (if
`expflag' was true). So if the parsing failed, then the value wasn't
added to the history. I'm attaching a patch with my changes.

My only doubt is that now, the call to `string_to_object' (which calls
`read-from-string', which calls `read1', and then `read0') is located
outside the context (not sure what the right term is) set up with
`specbind'. Could this be a problem? Do any of these functions depend on
the context set up in `read_minibuf'?  (They don't appear to, just want
to be sure).

Thanks.

[minibuf.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Federico Tedin <federicotedin <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 39291 <at> debbugs.gnu.org, pogonyshev <at> gmail.com, juri <at> linkov.net
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Thu, 30 Jan 2020 16:14:42 +0200

> From: Federico Tedin <federicotedin <at> gmail.com>
> Cc: Juri Linkov <juri <at> linkov.net>,  39291 <at> debbugs.gnu.org,  pogonyshev <at> gmail.com
> Date: Wed, 29 Jan 2020 22:32:16 +0100
> 
> I found that the minibuffer input string in `read_minibuf' was being
> added to the history list after we tried to parse an object from it (if
> `expflag' was true). So if the parsing failed, then the value wasn't
> added to the history. I'm attaching a patch with my changes.

Thanks.

> My only doubt is that now, the call to `string_to_object' (which calls
> `read-from-string', which calls `read1', and then `read0') is located
> outside the context (not sure what the right term is) set up with
> `specbind'. Could this be a problem? Do any of these functions depend on
> the context set up in `read_minibuf'?  (They don't appear to, just want
> to be sure).

Why is it a problem to move the call to unbind_to to after the call to
add-to-history?  AFAIU, the original code, before you changed it,
actually did that.  It just didn't call string_to_object, but I'm not
sure why would that change anything.  Does anyone see a problem here?
Stefan, any comments?

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: juri <at> linkov.net, pogonyshev <at> gmail.com, 39291 <at> debbugs.gnu.org,
 Federico Tedin <federicotedin <at> gmail.com>
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Thu, 30 Jan 2020 09:20:52 -0500

> Why is it a problem to move the call to unbind_to to after the call to
> add-to-history?

Because it would make `add-to-history` add to the minibuffer-local value
rather than to the callingbuffer-local value, as the comment explains:

  /* Add the value to the appropriate history list, if any.  This is
     done after the previous buffer has been made current again, in
     case the history variable is buffer-local.  */


-- Stefan

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 39291 <at> debbugs.gnu.org, pogonyshev <at> gmail.com,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Thu, 30 Jan 2020 09:23:11 -0500

> My only doubt is that now, the call to `string_to_object' (which calls
> `read-from-string', which calls `read1', and then `read0') is located
> outside the context (not sure what the right term is) set up with
> `specbind'. Could this be a problem? Do any of these functions depend on
> the context set up in `read_minibuf'?  (They don't appear to, just want
> to be sure).

AFAIK they don't depend on Lisp variables, no, so it should be safe to
move the call to `string_to_object` to after the `unbind_to`.


        Stefan

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

From: Federico Tedin <federicotedin <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 39291 <at> debbugs.gnu.org, pogonyshev <at> gmail.com,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Fri, 31 Jan 2020 01:44:34 +0100

> AFAIK they don't depend on Lisp variables, no, so it should be safe to
> move the call to `string_to_object` to after the `unbind_to`.
>
>
>         Stefan

Thanks for the information. In that case, the patch I sent should be the
right fix for this bug.

- Fede

Message #34 received at 39291-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: juri <at> linkov.net, pogonyshev <at> gmail.com, monnier <at> iro.umontreal.ca,
 39291-done <at> debbugs.gnu.org
Subject: Re: bug#39291: M-: history doesn't store erroneous input
Date: Fri, 31 Jan 2020 11:20:20 +0200

> From: Federico Tedin <federicotedin <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  39291 <at> debbugs.gnu.org,  pogonyshev <at> gmail.com,  Juri Linkov <juri <at> linkov.net>
> Date: Fri, 31 Jan 2020 01:44:34 +0100
> 
> > AFAIK they don't depend on Lisp variables, no, so it should be safe to
> > move the call to `string_to_object` to after the `unbind_to`.
> >
> Thanks for the information. In that case, the patch I sent should be the
> right fix for this bug.

Thanks, pushed to the release branch.

> * src/minibuf.c (read_minibuf): Parse input string after saving the
> string to the history list instead of before, in case parsing
> fails. (Bug#39291)
       ^^
Please in the future leave 2 blanks between sentences, per our
conventions.

Previous Next

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