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

[Drew Adams]

> Hi, Drew,

Hi Alan,

> I kind of feel myself being addressed here.  :-)

Consider it a compliment that I now see some use for this general feature -
beyond scrolling (which I personally won't use it for). ;-)

> > 1. The doc for this option, as well as its name, give the 
> > impression that it is
> > about allowing scrolling.  It is not.
> 
> It is largely about scrolling.

I meant that it is not _necessarily_ about scrolling.  I understand that that
was the original motivation, and it remains an important use case (for people
who want scrolling).

> > 1. For one thing, a non-nil value allows *any* command 
> >    bound to a key in `isearch-mode-map' to take advantage
> >    of a prefix arg.  That is, `C-u' is passed
> >    through to the command if the option value is non-nil.
> >    The command need not have anything to do with scrolling.
> 
> I'm not sure that's quite true.  Or if it is, keys like C-s 
> don't react to it.

AFAICT, it is 100% true.  For one thing, that's how
`isearch-query-replace(-regexp)' works.  Those commands do _not_ have property
`isearch-scroll' or `scroll-command' on their symbols.  They simply use the
feature that `C-u' gets passed through if `isearch-allow-scroll' is non-nil.

> > And there are already at least two such vanilla commands:
> > `isearch-query-replace' and `isearch-query-replace-regexp' 
> > (`M-%', `C-M-%').  A `C-u' changes the command behavior in
> > a way that has nothing to do with scrolling.
> 
> It's good that the C-u works, though.  ;-)

That's my point.  Like those commands, I want to make use of this `C-u' feature.
But I do not want to also allow scrolling during Isearch.

For example, I have an Icicles command (bound to `S-TAB') that runs from
Isearch, and if you use `C-u S-TAB' then its behavior is different.  But that
means that to take advantage of this users need to turn on
`isearch-allow-scroll'.

That would be OK if that only had the effect of allowing `C-u' pass-through.
But it also allows scrolling, and it's not obvious that users who want to use
`C-u S-TAB' also want Isearch to allow scrolling.

> > At a minimum, the doc should be adjusted.  The option name 
> > should also be changed to fit what it really does.
> 
> What would you suggest?  What does the option really do, in your
> judgement?  Scrolling is an essential part of the option.

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.  The function used to look
up such commands is `isearch-lookup-scroll-key', but there is nothing in that
function's definition that deals with scrolling or scroll commands (apart from
the names of the properties checked).

2. It passes a prefix arg through to any key (its command) that follows it.

Neither of these has anything to do with scrolling, necessarily.  Change the
names to remove `scroll' and you will see that.

Now, yes, it happens that the scrolling commands use property `scroll-command'.
That was one of my points: separate the property used to distinguish susceptible
commands for Isearch from a property that is associated with scrolling.  For
scrolling commands, if you want pass-through from Isearch then you just give
them property `isearch-scroll' (a bad name), in addition to property
`scroll-command'.

(And the code that gives the scrolling commands their Isearch behavior should
not even be in isearch.el, IMO.)

> > Likewise, the functions (e.g. `isearch-lookup-scroll-key') 
> > and other code and doc strings in isearch.el that paint this
> > feature as having to do with scrolling.
> 
> They're stricly internal functions, so changing their 
> names/doc strings wouldn't be a big deal.

Right, but my real point is not about the names but about freeing up the two
pass-through behaviors (one for specific commands, the other for `C-u') from
their coupling to scrolling.  Once freed, nothing would prevent you from
re-associating them: telling Isearch that you want scrolling commands to benefit
from the pass-through feature.

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

Now that does not mean that every command is a reasonable candidate for using
this feature.  It just means that the feature is not in any way
scrolling-specific.

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

Even if you are right about that, nothing says that that particular set of
commands is limited to scrolling commands.  And even if that were also the case,
nothing in fact prevents using an inappropriate command this way.  The code
looks for only one thing: property `isearch-scroll'.

> > 2. I would like to see us separate the treatment of a 
> >    prefix arg - whether it gets passed it through to a
> >    command or it exits Isearch - from the other
> >    uses/effects of this option.
> 
> I agree, that would be a good idea.  It wouldn't take very much work.

Very glad to hear that.

> Need it be an option?  Why not just let C-u through no matter what?

Dunno.  That would probably be OK for me, but I can imagine RMS or others
chiming in that they are used to having `C-u' immediately exit Isearch.  Having
an option means not disturbing the general, traditional behavior.

But as far as I'm concerned, I think I would be in favor of always letting
prefix-arg keys pass through without exiting Isearch.

(It would be up to the command they are passed through to to decide whether to
exit.  In the case of the Icicles feature that uses this, the command does in
fact exit Isearch, after picking up some Isearch info.  But the important thing
is to be able to pass a prefix arg to the command, which is bound to a key on
`isearch-mode-map'.)

> As a matter of interest, have you tried setting 
> isearch-allow-scroll?  If
> you have and didn't like it, what about it didn't you like?

I have tried it.  I don't recall just what I didn't like.  I wouldn't say that
I'm against it or I don't like it.  (For one thing, I'm not used to it.)  But I
do recall trying it out a fair amount a few years back.  You know, everyone is
different.  The fact that someone might not like to use it doesn't mean much.

> > 3. Wrt controlling which commands are affected by the 
> >    option (i.e., forgetting about `C-u' now): The current
> >    design makes the library designer responsible for
> >    this choice, not the user.  That is another flaw, IMO.
> >    A user should be able to easily (using Customize, not just
> >    putting `put' here and there) choose which commands are affected.
> 
> I disagree totally here.  The sophisticated user can set a suitable
> command to be a "scrolling" command.  The unsophisticated 
> user is going to get his Emacs into a thorough mess by messing
> around in this area.

I suspect you are still thinking of this in terms of scroll commands.  I am not.

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.

Currently, it is all or nothing: all scroll commands plus any others that might
have property `isearch-scroll' or else none of them.

Especially if we open this up to intentionally be about pass-through in general
and not just scrolling, users should have a way to easily pick and choose which
commands are affected by it.  (I say "intentionally" because it is already open
but without any advertising and partially hidden behind the names "scroll"
this-and-that.

> > Instead of a Boolean option (`isearch-allow-scroll'), users 
> > should have an option that specifies the affected commands.
> > (It could also configure any specifics for each command, if
> > there are such.)
> 
> Again, familiarise yourself with just how restricted the permissible
> commands are.  There's a list of criteria in isearch.el 
> ~L1760 (think - number of yards in a mile :-).

I read that comment, and I read the code.  Again, I think you are thinking only
in terms of scrolling commands.  The feature is in fact only about pass-through
to a command, letting it run without necessarily exiting Isearch.  It is just
like the `C-u' pass-through, but without the `C-u'.  The code is not
"scroll"-specific at all, AFAICT.

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.

My point about the non-C-u pass-through was mainly to say that it is already
there, and it has nothing, a priori, to do with scrolling.  That its predefined
use might currently be limited to scrolling does not mean that the code actually
recognizes scrolling.  Likewise, if it turns out that no one will ever want to
use it for any command other than scrolling.  The code itself is agnostic -
scrolling-blind (AFAICT).

> > 4. Why are there currently _two_ different properties that 
> >    turn on this sensitivity (i.e., "scrolling")?  From the
> >    code comments:
> 
> > ;; ... property called `isearch-scroll'.
> > ;; If a command's symbol has the value t for this property 
> > ;; or for the `scroll-command' property, it is a scrolling
> > ;; command.
> 
> I haven't a clue.  I've no idea who put `scroll-command' in or why.

I presume that it is for scrolling commands, outside of any Isearch context.

> > This means that if someone adds property `scroll-command' 
> > for some command then it automatically acts as if s?he
> > also added property `isearch-scroll'.  Why couple these
> > two things?  Why assume that every `scroll-command' command
> > is also one for which Isearch should allow scrolling.
> 
> Pretty much all standard commands that can be "scrolling" commands
> already are.  Let me know if there are any I have missed.

I know.  My question was why.

But I don't really care.  My point was that these are two different things that
need not be automatically coupled.  They can instead be explicitly coupled if
that's a good idea.  IOW, instead of having Isearch automatically recognize
property `scroll-command', just use property `isearch-scroll' for all pertinent
commands (even if they already have property `scroll-command').

Again, though.  I don't want to insist on using pass-through for non-scrolling
commands.  I don't really care about that.

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.

Passing `C-u' through to a command (e.g. `isearch-query-replace') really has
nothing to do, logically, with allowing scrolling.  Users who would like to
allow `C-u' pass-through should not also have to allow scrolling.  That's my
main point, and I think you agree with that much.