Subject: bug#49963: 28.0.50; isearch failing in Dired after rectangle-mark-mode and query-replace

From: Eli Zaretskii <[email-masked]>
Date: Wed, 11 Aug 2021 15:39:39 +0300

> From: Stefan Monnier <[email-masked]>
> Cc: [email-masked],  [email-masked],  [email-masked],
>   [email-masked]
> Date: Tue, 10 Aug 2021 16:37:22 -0400
> 
> Eli Zaretskii [2021-08-10 19:42:17] wrote:
> >> From: Stefan Monnier <[email-masked]>
> >> Cc: Eli Zaretskii <[email-masked]>,  [email-masked],  [email-masked],
> >>   [email-masked]
> >> Date: Tue, 10 Aug 2021 11:56:58 -0400
> >> 
> >> Lars Ingebrigtsen [2021-08-10 17:44:50] wrote:
> >> > Eli Zaretskii <[email-masked]> writes:
> >> >> I meant can we please not use add-function and friends.  Please?
> >> > There isn't much difference between using add-function and add-hook most
> >> > of the time (although here we trip over a `let' binding not doing what
> >> > the author thought it did).
> >> 
> >> FWIW, `add-hook` has the exact same problem with `let` (and we already
> >> tripped against this exact same situation with `add-hook` and `let`).
> >
> > This is a misunderstanding of what bothers me.  The problem is
> > discoverability: add-function is not easily discoverable, if it is
> > used on an internal function.
> 
> I'm sorry, I don't understand.  AFAIK it's used on variables (in the
> present case it's used on the variable `isearch-filter-predicate`)
> holding functions, not on internal functions.

And if I've said instead

  This is a misunderstanding of what bothers me.  The problem is
  discoverability: add-function is not easily discoverable, if it is
  used on an internal variable.

then you'd understand?  If so, please excuse my silly typo, and please
reply to the amended text above.

> > IOW, the documentation of C-s doesn't tell you that its operation
> > could be affected by that "hook".
> 
> You mean the doc of `C-s` should state that it's affected by
> `isearch-filter-predicate`?
> 
> Fine by me, but I don't know what this has to do with `add-function`
> (`isearch-filter-predicate` existed before `add-function` was invented).

It makes the rabbit hole much deeper and darker.  Because even if and
when I find that isearch-filter-predicate is involved in this (which
isn't easy, see below), its "C-h v" shows the following gobbledygook:

  isearch-filter-predicate is a variable defined in ‘isearch.el’.

  Its value is
  #f(advice-wrapper :after-while #f(compiled-function
				    (&rest args)
				    #<bytecode 0x18003e4619de40ca>)
		    wdired-isearch-filter-read-only)

And that's the second try, after "M-: isearch-filter-predicate RET",
a standard way of figuring out the values of variables,which curses
thusly:

  #[128 "\300\302\"\205^@\300\301\"\207" [apply wdired-isearch-filter-read-only #[128 "\301\302\300!\"\207" [isearch-filter-predicate apply default-value] 4 "
  
  (fn &rest ARGS)"] nil] 4 nil]

Why does wdired need to use add-function? why couldn't it simply put
its own function on isearch-filter-predicate's value?

The code in isearch.el is notoriously hard to debug, because it is
written in many layers, uses a lot of function variables and indirect
calls, and almost none of its important subroutines have any useful
doc strings or comments.  And stepping with Edebug through its code is
also not easy because of the way Isearch reads input, which conflicts
with Edebug's SPC-stepping.  So tracking an issue related to
isearch.el is an endless mess of guessing the next candidate for being
part of the puzzle, following the chain of indirect calls through
variables and hooks, then finding the next suspect, etc. etc.

Using add-function on top of that makes a bad problem much worse.  So
I'm asking whether we could improve that by not using add-function.
Bonus points for adding meaningful doc strings and/or comments to
important isearch.el functions and variables so that one could easier
find the possible suspects by following the code and the documentation
instead of stepping blindly through complex code.  TIA.