bug#69056: 30.0.50; history-add-new-input and recursive minibuffers

[Eshel Yaron]

Stefan Monnier <monnier@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
Description: Text Data