* bug#39291: M-: history doesn't store erroneous input
@ 2020-01-26 15:16 Paul Pogonyshev
2020-01-28 23:35 ` Juri Linkov
0 siblings, 1 reply; 10+ messages in thread
From: Paul Pogonyshev @ 2020-01-26 15:16 UTC (permalink / raw)
To: 39291
[-- Attachment #1: Type: text/plain, Size: 323 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 454 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-26 15:16 bug#39291: M-: history doesn't store erroneous input Paul Pogonyshev
@ 2020-01-28 23:35 ` Juri Linkov
2020-01-29 17:16 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2020-01-28 23:35 UTC (permalink / raw)
To: Paul Pogonyshev; +Cc: 39291
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-28 23:35 ` Juri Linkov
@ 2020-01-29 17:16 ` Eli Zaretskii
2020-01-29 18:09 ` Federico Tedin
2020-01-29 21:32 ` Federico Tedin
0 siblings, 2 replies; 10+ messages in thread
From: Eli Zaretskii @ 2020-01-29 17:16 UTC (permalink / raw)
To: Juri Linkov, Federico Tedin; +Cc: 39291, pogonyshev
> From: Juri Linkov <juri@linkov.net>
> Date: Wed, 29 Jan 2020 01:35:33 +0200
> Cc: 39291@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-29 17:16 ` Eli Zaretskii
@ 2020-01-29 18:09 ` Federico Tedin
2020-01-29 21:32 ` Federico Tedin
1 sibling, 0 replies; 10+ messages in thread
From: Federico Tedin @ 2020-01-29 18:09 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 39291, pogonyshev, Juri Linkov
[-- Attachment #1: Type: text/plain, Size: 923 bytes --]
On Wed, 29 Jan 2020 at 18:16 Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Juri Linkov <juri@linkov.net>
> > Date: Wed, 29 Jan 2020 01:35:33 +0200
> > Cc: 39291@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.
>
[-- Attachment #2: Type: text/html, Size: 1902 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-29 17:16 ` Eli Zaretskii
2020-01-29 18:09 ` Federico Tedin
@ 2020-01-29 21:32 ` Federico Tedin
2020-01-30 14:14 ` Eli Zaretskii
2020-01-30 14:23 ` Stefan Monnier
1 sibling, 2 replies; 10+ messages in thread
From: Federico Tedin @ 2020-01-29 21:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 39291, pogonyshev, Juri Linkov
[-- Attachment #1: Type: text/plain, Size: 709 bytes --]
> 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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1310 bytes --]
From 6a35a1390d1f1924d0fd52dbb885af66d089273c Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Wed, 29 Jan 2020 22:24:40 +0100
Subject: [PATCH 1/1] Ensure input is added to history in read_minibuf
* 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)
---
src/minibuf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/minibuf.c b/src/minibuf.c
index 8ebdff1252..9d870ce364 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -697,10 +697,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
else
histstring = Qnil;
- /* If Lisp form desired instead of string, parse it. */
- if (expflag)
- val = string_to_object (val, defalt);
-
/* The appropriate frame will get selected
in set-window-configuration. */
unbind_to (count, Qnil);
@@ -711,6 +707,10 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
if (! (NILP (Vhistory_add_new_input) || NILP (histstring)))
call2 (intern ("add-to-history"), histvar, histstring);
+ /* If Lisp form desired instead of string, parse it. */
+ if (expflag)
+ val = string_to_object (val, defalt);
+
return val;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-29 21:32 ` Federico Tedin
@ 2020-01-30 14:14 ` Eli Zaretskii
2020-01-30 14:20 ` Stefan Monnier
2020-01-30 14:23 ` Stefan Monnier
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2020-01-30 14:14 UTC (permalink / raw)
To: Federico Tedin, Stefan Monnier; +Cc: 39291, pogonyshev, juri
> From: Federico Tedin <federicotedin@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>, 39291@debbugs.gnu.org, pogonyshev@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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-30 14:14 ` Eli Zaretskii
@ 2020-01-30 14:20 ` Stefan Monnier
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2020-01-30 14:20 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 39291, Federico Tedin, pogonyshev, juri
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-29 21:32 ` Federico Tedin
2020-01-30 14:14 ` Eli Zaretskii
@ 2020-01-30 14:23 ` Stefan Monnier
2020-01-31 0:44 ` Federico Tedin
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2020-01-30 14:23 UTC (permalink / raw)
To: Federico Tedin; +Cc: 39291, pogonyshev, Juri Linkov
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-30 14:23 ` Stefan Monnier
@ 2020-01-31 0:44 ` Federico Tedin
2020-01-31 9:20 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Federico Tedin @ 2020-01-31 0:44 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 39291, pogonyshev, Juri Linkov
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#39291: M-: history doesn't store erroneous input
2020-01-31 0:44 ` Federico Tedin
@ 2020-01-31 9:20 ` Eli Zaretskii
0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2020-01-31 9:20 UTC (permalink / raw)
To: Federico Tedin; +Cc: 39291-done, pogonyshev, monnier, juri
> From: Federico Tedin <federicotedin@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 39291@debbugs.gnu.org, pogonyshev@gmail.com, Juri Linkov <juri@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-01-31 9:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-26 15:16 bug#39291: M-: history doesn't store erroneous input Paul Pogonyshev
2020-01-28 23:35 ` Juri Linkov
2020-01-29 17:16 ` Eli Zaretskii
2020-01-29 18:09 ` Federico Tedin
2020-01-29 21:32 ` Federico Tedin
2020-01-30 14:14 ` Eli Zaretskii
2020-01-30 14:20 ` Stefan Monnier
2020-01-30 14:23 ` Stefan Monnier
2020-01-31 0:44 ` Federico Tedin
2020-01-31 9:20 ` Eli Zaretskii
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.