* 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.