bug#18241: 24.4.50; [PATCH] I can now highlight-lines-matching-regexp from isearch

[Eli Zaretskii <eliz@gnu.org>]

> From: Dima Kogan <dima@secretsauce.net>
> Date: Sun, 30 Jun 2019 20:09:59 -0700
> Cc: 18241@debbugs.gnu.org, Lars Ingebrigtsen <larsi@gnus.org>

Thanks, some comments regarding the documentation parts:

> +@kindex M-s h l @r{(Incremental Search)}
> +@findex isearch-highlight-lines-matching-regexp
> +  You can exit the search while leaving matches for the last search
> +string highlighted by typing @kbd{M-s h r}
> +(@code{isearch-highlight-regexp}).  This runs @code{highlight-regexp}
> +(@pxref{Highlight Interactively}), passing it the regexp derived from
> +the last search string and prompting you for the face to use for
> +highlighting.  Similarly, you can highlight whole lines containing
> +matches by typing @kbd{M-s h l}
> +(@code{isearch-highlight-lines-matching-regexp}).

This description left me wondering what is the difference between
these two commands.  IOW, how does "last search string" and "whole
lines containing matches" differ from one another?  The problem is
probably with using "search string" instead of "match" in the
description of the first command, but that's a guess.

> +'M-s h l' invokes highlight-lines-matching-regexp directly using the
> +search string, similar to what 'M-s h r' was doing already.

This needs to be more clear.  "Similar" in what sense? and if it's
similar enough, why did we introduce a new command/key binding?  Also,
since the new command is already described in the manual, this entry
should be marked with "+++", see the beginning of NEWS for
explanations why.

> -(defun isearch-highlight-regexp ()
> +(defun isearch--highlight-regexp-or-lines (hi-lock-func)
>    "Run `highlight-regexp' with regexp from the current search string.

We prefer the first line of the doc string to mention the arguments.

> +(defun isearch-highlight-regexp ()
> +  "Run `highlight-regexp' with regexp from the current search string.

What is "current search string"?  This should be explained in the rest
of the doc string, but isn't.

> +It exits Isearch mode and calls `hi-lock-face-buffer' with its regexp
> +argument from the last search regexp or a quoted search string,

I'm guessing "the last search regexp or a quoted search string" refers
to the same string as "the current search string" in the first line.
If so, please note that it is generally a bad idea to describe the
same thing by more than one term, because it leaves the reader
wondering whether you indeed mean the same thing.

> +(defun isearch-highlight-lines-matching-regexp ()
> +  "Run `highlight-lines-matching-regexp' with regexp from the
> +current search string.  It exits Isearch mode and calls

The first line of the doc string should be a complete sentence.

Thanks.