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

[Alan Mackenzie]

Hi, Drew.

On Sat, Sep 10, 2011 at 09:43:23AM -0700, Drew Adams wrote:
> > 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.

Dash it, Drew, you can try a man's patience.  It was me who programmed
the device, me who first used "scrolling command", so I get to define
what it means.  I know you don't like the term, but if you can't bring
yourself to use it as I've defined it, please suggest another term for
it.  If we can't agree on terminology, then there's not much point trying
to carry out a discussion.

> 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.

I'm kind of with Stephen at the moment.  What exactly do you mean by
"command pass-through"?

> ;; Bound to `S-TAB' in Isearch.

OK.  Things bound in the isearch-mode-map are a different kettle of fish
entirely from scrolling commands.

> (defun icicle-search-w-isearch-string (&optional use-context-p)
>   (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))))

[ .... ]

> > 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.

Ah right, got you.  I thought you were talking about the executable code
dealing with the property, not the setting of it.

> > 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_.

I couldn't disagree more, if we're talking about something like
`recenter-top-bottom'.  If we're talking about a command bound in
isearch-mode-map, like `icicle-s-w-i-s', then yes.

> > > > > 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).

I don't know what you mean.  What is passing a command through what to what?

> 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.

We're agreed on this point.  My patch was trying to achieve that.

> 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.

The definition of "scrolling command" prevents it.  It would be
profoundly bad programming practice for a random command to manipulate
the internals of isearch.  For a command on the isearch-mode-map (which
cannot be a "scrolling command") it's a different matter.

[ .... ]

> 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.

Why?  With the exception of the prefix-arg commands, you haven't made any
strong case for this.  Can you give an example of two scrolling commands,
one of which a user would strongly want, the other strongly not want?

> 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 is surely enough.  I can't conceive of any situation where the
binary choice is inadequate.  What commands might a user want to separate
out?

> > Try out this patch; I think it does what you want.

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

No.  I must have been 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'.

OK.  It was worth a try.  :-)

Anyhow, we're in feature freeze at the moment, so until Emacs 24 is split
off onto a branch, there's nowhere for fresh code to go.

-- 
Alan Mackenzie (Nuremberg, Germany).