Re: Simple isearch concerns

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 08 Apr 2021 22:40:42 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: Ergus <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?

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

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

> +(defcustom isearch-allow-match-scroll nil
> +  "Whether scrolling to another match is allowed during incremental search.
              ^^^^^^^^^
  Whether to allow movement between isearch matches by cursor motion commands.

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

> +See also the related option `isearch-allow-scroll'."
> +  :type '(choice (const :tag "Off" nil)
> +                 (const :tag "On" t))
> +  :group 'isearch)

The :version tag is missing here.

> +     ;; Handle match scrolling functions.

Again, "scrolling" is the wrong terminology for describing this.

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

Thanks.