bug#65459: completing-read INITIAL-VALUE unaware of COLLECTION and REQUIRE-MATCH

[Stefan Monnier]

>>> (defun my-completing-read (prompt collection start)
>>>   (minibuffer-with-setup-hook
>>>       (:append
>>>        (lambda ()
>>>          (setq-local actual-minibuffer-default-add-function
>>>                      minibuffer-default-add-function)
>>>          (setq-local minibuffer-default-add-function
>>>                      '(lambda () (setq minibuffer-default-add-done nil)
>>>                         (funcall actual-minibuffer-default-add-function)))
>>
>> Why use ' on `lambda`?
>>
>
> I think my finger slipped.
>
>>
>> BTW, rather than the above two `setq-local`s, I think you can write:
>>
>>    (add-function :before (local 'minibuffer-default-add-function)
>>                  (lambda () (setq minibuffer-default-add-done nil)))
>>
>
> Right.  I was writing that function as it if were for minibuffer.el, and was
> therefore avoiding advices.  But now I see that there are a couple of
> add-function in core anyway (it's not clear to me where the dividing line
> is).  So here's the final version:

The issue is not "don't use `(n)advice.el`", but "don't modify functions
on the sly".  And by "functions" this refers to those things stored in
the `symbol-function` slot of symbols in the global obarray.  This is
because code that does `(my-foo ...)` usually expects to execute the
code found in `(defun my-foo ...)` and not something else, and readers
of that code often make the same assumption, so it can make debugging
really nasty.

Changing a function with `fset` (or `cl-letf` or `defalias`) is worse
than using `advice-add`, for that reason: at least `advice-add` sets up
the help system such that `C-h f` will (hopefully) warn you about the
presence of an advice.

For variables containing functions (such as
`minibuffer-default-add-function`), there is no such expectation that
calling this function will run some known piece of code, on the
contrary: the whole point of the variable is to run difference pieces of
code in different contexts, so modifying the function is perfectly OK,
regardless whether you do it with `setq` or `add-function`.  Here I'd
use `add-function` because it's simpler.


        Stefan