Re: Simple isearch concerns

[Gregory Heytings]

Thanks for your detailed comments!

> > +@vindex isearch-allow-match-scroll
>
> IMO, the name of this variable (and the corresponding property) is not 
> the best one.  "Scroll" is the wrong term here, and "match-scroll" 
> doesn't help understanding what it does.  How about 
> isearch-allow-motion-commands instead?

I tend to agree with you, but I'm not sure.  There is already an 
'isearch-allow-scroll' and its corresponding 'isearch-scroll' property, 
which changes the behavior of motion commands: it becomes possible to 
scroll (recenter, scroll-up, scroll-down, ...) but without changing the 
current match.  This patch does something related, but different: it 
becomes possible to scroll among the matches.

> > +@cindex motion commands, during incremental search, change
> > +  When @code{isearch-allow-match-scroll} is non-@code{nil}, it
> > +is also possible to change the effect of motion commands during
> > +incremental search, by modifying the @code{isearch-match-scroll}
> > +property of their symbols.  For example, to make @kbd{C-p} and
> > +@kbd{C-n} move to the previous and next line and restart Isearch
> > +forward and backward respectively, you can put the following
> > +lines in your init file (@pxref{Init File}):
> > +
> > +@example
> > +(put 'next-line 'isearch-match-scroll '(next-line . forward))
> > +(put 'previous-line 'isearch-match-scroll '(previous-line . backward))
> > +@end example
> >  @end table
>
> This is too "Lispy" IMO to be in the user manual.  I suggest to move the 
> bulk of it into the ELisp manual, and leave only a reference to that in 
> the user manual.

Okay.  I've put it there because just above the 'isearch-move' and 
'isearch-scroll' properties are discussed, but indeed no example is given, 
and these properties are only t/nil ones.

> > ++++
> > +** New user option 'isearch-allow-match-scroll'.
> > +When this option is set, the commands 'beginning-of-buffer',
> > +'end-of-buffer', 'scroll-up-command' and 'scroll-down-command' move
> > +respectively to the first occurrence of the current search string
> > +in the buffer, the last one, the first one after the current window,
> > +and the last one before the current window.
>
> This sentence should make it clear that it talks about the named 
> commands that are invoked while in I-search.  Otherwise the sentence is 
> misleading.

I'm not sure I understand what you mean.  How would you make it clearer?

> > +If non-nil, the four scrolling commands `beginning-of-buffer',
> > +`end-of-buffer', `scroll-up-command' and `scroll-down-command' move
> > +respectively to the first first occurrence of the current search string in
> > +the buffer, the last one, the first one after the current window, and the
> > +last one before the current window.
>
> Presumably if this option is nil, these commands exit I-search and are
> executed?  This should be in the doc string.

Indeed, if this option is nil, the meaning of these commands is unchanged, 
which means that they exit Isearch.  Should I add something like "If this 
option is nil, the meaning of these commands is unchanged"?  But that 
could be misleading, because that's not correct when isearch-allow-scroll 
is non-nil...

> > +        (condition-case nil
> > +            (funcall function)
> > +          (error nil))
>
> Wouldn't it be better to at least display the error message (or some 
> message) in the case the function signals an error?

In this specific case, no: what happens is that Isearch indicates that the 
search will wrap.