GNU bug report logs - #69056
30.0.50; history-add-new-input and recursive minibuffers

Previous Next

Package: emacs;

Reported by: Eshel Yaron <me <at> eshelyaron.com>

Date: Sun, 11 Feb 2024 15:56:02 UTC

Severity: normal

Found in version 30.0.50

Full log


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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 69056 <at> debbugs.gnu.org
Subject: Re: bug#69056: 30.0.50; history-add-new-input and recursive
 minibuffers
Date: Thu, 15 Feb 2024 19:40:29 +0100
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Thanks, that's what I thought too.  Here's an attempt do just that:
>
> Looks pretty good.  I do have some comments/questions:
>
>> @@ -902,6 +903,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
>>    /* Don't allow the user to undo past this point.  */
>>    bset_undo_list (current_buffer, Qnil);
>>
>> +  /* Cache the buffer-local value. */
>> +  nohist = NILP (find_symbol_value (Qhistory_add_new_input));
>
> Why not use `Vhistory_add_new_input`?

Good question, I guess for some reason I assumed that `NILP (Vfoo)`
doesn't check the buffer-local value like `find_symbol_value (Qfoo)`
does...

> [ Also, it's not really "cache" (which implies it impacts only
>   performance).  More like "remember".  ]

>> @@ -965,7 +969,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
>>    /* 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.  */
>> -  if (! (NILP (Vhistory_add_new_input) || NILP (histstring)))
>> +  if (! (nohist || NILP (histstring)))
>>      call2 (Qadd_to_history, histvar, histstring);
>>
>>    /* If Lisp form desired instead of string, parse it.  */
>
> IIUC this change is needed because by the time we get here the
> buffer-local value of `history-add-new-input` has been flushed by
> `minibuffer-inactive-mode` called by `read_minibuf_unwind`,
> itself run by the `unbind_to` a few lines above.
> So maybe we can simplify this by just moving the above 2 lines before
> the `unbind_to`, WDYT?

Oh, that's much simpler indeed.  And it seems to work just as well.
Here's an updated patch (v2):

[v2-0001-Use-buffer-local-value-of-history-add-new-input-i.patch (text/x-patch, attachment)]

This bug report was last modified 1 year and 176 days ago.

Previous Next


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