bug#20063: 24.4: read-from-minibuffer improperly setting hist parameter

[Boruch Baum]

On 03/11/2015 10:09 AM, Stefan Monnier wrote:
>> but the benefit of this to the end-user is very limited, and has
>> a downside if done simply.
> 
> If the benefit is limited, it means the problem you mention is
> correspondingly minor.
I was referring to the benefit of your idea to filter out !COLLECTION
elements dynamically, not the bug that offers the user nonsense or
unacceptable HIST elements in the mini-buffer.

>> Once REQUIRE-MATCH=t, nothing but elements of COLLECTION will ever be
>> accepted, so `concat'-ing the filtered elements of HIST would present
>> only legitimate options, in the sequence most recently used, but with
>> potentially a lot of duplicate entries.
> 
> I'm not sure exactly what you mean here, but I suspect you assume
> COLLECTION to be finite and small.
The characterization of 'finite' isn't an assumption; it's a requirement
- how could one REQUIRE-MATCH=t against a COLLECTION of infinite size?
OTOH, your characterization of 'small' (and the meaning of 'small' is
always difficult) isn't an assumption of mine, but, who knows, it may
have been an assumption of the designers of the mini-buffer functions.

>> Using `add-to-list', starting with an empty list would avoid
>> the duplications and present the elements in sequence
>> most-recently-used.
> 
> Duplicate elements in the history are yet again orthogonal.
> You probably want to set history-delete-duplicates to t.
I wasn't advising what I want; I was trying to be helpful by pointing
out a problem in the mini-buffer function. A user may normally want to
retain duplicates in her general command history as a record of past
activity, but not have those duplicates appear in mini-buffer selections
that have REQUIRE-MATCH=t.

To illustrate, imagine yourself, as a user, scrolling back through a
minbuffer history in order to see what your legitimate REQUIRE-MATCH=t
options are. When would you ever want duplicates to appear in your
scrolling? It would only delay your ability to see all your options.

>>> does provide the option of "no history".
>> Which brings us full-circle to line 974 of `minibuf.c'
> 
> I don't understand this, since this code checks for a nil value, not
> a t value.
My report never discussed the undocumented HIST=t option; that was your
contribution. The documentation does provide for HIST=nil, which -IS- a
central element of the bug report.

We've been covering a lot of ground, so its understandable that we may
have started straying from the original bug report (see there), but that
also can be useful and constructive if it helps inform a resolution.
This is especially true since we're discussing very widely used functions.

It may be helpful to look at the issue from two perspectives:

1] 'bottom-up', starting from `read-from-minibuffer'. This would be the
theoretical perspective;

2] 'top-down', starting from functions such as `toggle-option' and
`highlight-regexp'. This would be the practical perspective.


-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

signature.asc
Description: OpenPGP digital signature