RE: `isearch-allow-scroll' - a misnomer and a bad design

[Drew Adams]

> > AFAICT, it does two things:
> 
> > 1. It invokes any command that has property 
> `isearch-scroll' or `scroll-command'
> > on its symbol - without necessarily exiting Isearch.
> 
> NO NO NO!!!  A "scrolling command" is a command which MOST DEFINITELY
> DOESN'T exit the isearch.  This is the definition of 
> "scrolling command" in isearch.  With this in mind, please think
> through everything you've written.

Well, yes and no.  Yes, in general Isearch tries to prevent exit.

But if a command wants to exit, it can do so:

(defun foo ()
  (interactive)
  (isearch-done)
  (message "WWWWWWWW")(sleep-for 2)
  (forward-char 2))

That's what I do now, to exit Isearch and invoke Icicles search.  In this case,
I use only `C-u' pass-through, not command pass-through.

;; Bound to `S-TAB' in Isearch.
(defun icicle-search-w-isearch-string (&optional use-context-p)
  "Icicles-search the buffer using an Isearch string chosen by completion.
The string you choose is used as the Icicles search context.  You can
navigate among its occurrences or search within those occurrences for
a subpattern.
If option `isearch-allow-scroll' is non-nil then a prefix arg changes
the behavior, as follows:
1. You are prompted for an Icicles search-context regexp.
2. You are prompted for an Isearch string, which you choose using
   completion.  It is copied to the `kill-ring'.
3. You can yank that string anytime during Icicles search, to search
   for it within the contexts defined by the regexp matches."

  (interactive "P")
  (isearch-done)
  (if (not use-context-p)
      (icicle-search (point-min) (point-max)
                     (icicle-isearch-complete-past-string) t)
    (let ((regexp  (icicle-search-read-context-regexp)))
      (kill-new (icicle-isearch-complete-past-string))
      (icicle-search (point-min) (point-max) regexp t))))

> > 2. It passes a prefix arg through to any key (its command) 
>      that follows it.
> 
> Bear in mind that C-u `universal-argument' is a scrolling 
> command in its own right.  So we can construe what you're
> asking for thusly: to have an option for a subset of scrolling
> commands to be executable in isearch

If you like.  I really don't care how you characterize it, as long as it is
understood.

Users of a command such as `icicle-search-w-isearch-string' should not need to
also allow _scrolling_ for Isearch, just to allow such a command to work from
Isearch, picking up the prefix arg.  IOW, I would like to be able to remove this
part of the command's doc string: "If option `isearch-allow-scroll' is non-nil".
This command has nothing to do with scrolling within Isearch.  Absolutely
nothing.

> > Neither of these has anything to do with scrolling, necessarily.
> > Change the names to remove `scroll' and you will see that.
> 
> [ .... ]
> 
> > (And the code that gives the scrolling commands their 
> > Isearch behavior should not even be in isearch.el, IMO.)
> 
> I'm not sure what you mean by this.

It's a detail - not important.  IMO, it might be better to put this kind of code
at the place where the command is defined, and not in isearch.el:
(put 'recenter 'isearch-scroll t)
But I certainly won't argue the point.

> Their "isearch behaviour" is part of isearch.  These commands
> operate precisely the same way, regardless of
> whether they're called from isearch or not.

The fact that you want some particular command to not exit Isearch is part of
the behavior you want for _that command_.  Yes, it's the part of the behavior
that is within an Isearch context... so it's a choice.  I would probably put the
`put' where the command is defined and not where Isearch is defined.  But it's a
choice.

The same kind of thing arises for decisions about where to put thing-at-point
properties: at the point of the function's definition, or in thingatpt.el?  And
so on.  We can perhaps agree to disagree about this.

> > > > 1b. For another thing (i.e., forgetting about `C-u'), *any*
                ^^^^^^^              ^^^^^^^^^^^^^^^^^^^^^^  
> > > >     command can benefit from the same Isearch feature as a
> > > >     scrolling command.  It suffices to put a non-nil
> > > >     `isearch-scroll' property on the command symbol.
> >
> > > This isn't true.
> >
> > It is true, AFAICT.  Nothing prevents you from putting 
> > property `isearch-scroll' on *any* command, to get Isearch
> > to pass through to it.
> 
> The current isearch, with isearch-allow-scroll set, will pass a C-u
> through to the next command regardless of whether that command is a
> scrolling command.

You changed the subject.  (1b) is about passing a _command_ through, not passing
`C-u' through (see: "forgetting about `C-u'", above).

But wrt what you say: Yes, `C-u' is passed through regardless of whether the
command is a "scrolling" command.  I do not put property `isearch-scroll' (or
`scroll-command') on command `icicle-search-w-isearch-string'.  And it works.

But what is important (to me) is that users must turn on `isearch-allow-scroll'
to enable the `C-u' pass-through.  They should not have to.  I myself do not
want scrolling within Isearch (I want `C-v' to exit Isearch, for example), yet I
want to be able to use `C-u S-TAB' from within Isearch.

> > > Only commands which don't foul up the isearch can be
> > > permitted to use the feature.  Only a few commands meet 
> > > the criteria.  `universal-argument' is such a command.
> 
> > (It would be up to the command they are passed through to 
> > to decide whether to exit.
> 
> That would be something entirely new.  It might involve such a command
> returning t/nil to instruct isearch to exit or not.  But that 
> would be a horrible kludge.

One person's horrible kludge is another's feature. ;-)

But as I say, I personally don't have a need (so far) for command pass-through.
`C-u' pass-through would be enough, for me.

The point here was that the code _already_ is general and not in any way limited
to "scrolling", in spite of the "scrolling" vocabulary used to describe it.
Nothing prevents a "scrolling command" (that is, a command with property
`isearch-scroll') from exiting Isearch - it is up to the command.

> (Every command being adjusted to do this?  

What "every command"?  I'm simply describing what the code already does.  I'm
not describing some wishlist feature.

> What about other uses of the return value?  There are an awful lot of
commands)

No idea what you're on about here.  I think you have perhaps misunderstood me.

> > Granted that users might not want to mess around with the 
> > scrolling commands (whether via `put' or Customize), perhaps
> > you will recognize that a user might want to not have Isearch
> > pass through to some command that library `foo' thinks
> > it is a great idea to pass through to by default.
> 
> You mean "pass through a C-u"?

No, this is still part of (1b): _command_ pass-through, not `C-u' pass-through.

The point of that sentence is to say that _users_ should have the control, not
some library foo.el that might baptise some command `foo' as a "scrolling"
command (i.e., pass-through).

A user should be able to allow pass-through for some commands and not others.
Today, s?he can do that only by allowing all of them (non-nil
`isearch-allow-scroll') and then removing `isearch-scroll' or `scroll-command'
properties from the commands s?he does not want to pass through.

> > That said, I don't want to belabor this part.  My main 
> > interest is in freeing up `C-u', which you have already agreed to.
>   [...]
> > I would like to see `C-u' passed through - that's the main 
> > point.  Either systematically or optionally, but if optionally
> > then separately from allowing scrolling.
> 
> Try out this patch; I think it does what you want.

I don't think so.  Am I missing something?

(defun foo (arg)
  (interactive "P")
  (isearch-done)
  (message "arg: %S" arg)(sleep-for 2)
  (forward-char 2))

(define-key isearch-mode-map "\C-f" 'foo)

Leave `isearch-allow-scroll' nil.
Now search and hit `C-u C-f'.

AFAICT, the `C-u' exits Isearch.  It is not passed through to `foo'.