Re: Simple isearch concerns

[Eli Zaretskii]

> Date: Fri, 09 Apr 2021 07:20:14 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: juri@linkov.net, spacibba@aol.com, emacs-devel@gnu.org
> 
> 
> >> +@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.

The "scroll" part is what I think needs to be amended: the commands
you are affecting are not scroll commands, they are cursor motion
commands.

> >> ++++
> >> +** 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?

I would say "the commands 'beginning-of-buffer', ..., when invoked
during I-search, move respectively to ...".

> >> +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"?

No, just that they exit the search and are executed normally.

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

I don't understand: why are you wrapping the call with condition-case,
in that case?