bug#49120: [PATCH] Add commands 'kill-lines' and 'copy-lines'

[Eli Zaretskii]

> Date: Sat, 19 Jun 2021 17:12:25 +0000
> From:  Okam via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> These commands work similarly to the 'flush-lines' command, but add the
> lines to the kill ring as a single item.

Thanks.  However, the "as a single item" part is not clear enough and
should be clarified.  Do you mean "as a single string"?  If so, I
would suggest to say that, and also explicitly say that the string
includes the newlines between the lines.

> +@findex kill-lines
> +@item M-x kill-lines
> +Like @code{flush-lines}, but also add the matching lines to the kill
> +ring as a single item.

I'd suggest to reword:

  Like @code{flush-lines}, but also add the matching lines to the kill
  ring.  The command adds the matching lines to the kill ring as a
  single string, including the newlines that separated the lines.

The reason I think it's better to separate this into two sentences is
that "also" is only relevant to the first part, not the second.

> +(defalias 'kill-matching-lines 'kill-lines)
> +(defalias 'copy-matching-lines 'copy-lines)

I wonder why we need these aliases, and in fact why not have only
kill-matching-lines without the shorter kill-lines?  The latter omits
the crucial reference to the "matching" part, and is too similar to
kill-word, kill-paragraph, etc.

>  (defalias 'count-matches 'how-many)
>  
>  (defun keep-lines-read-args (prompt)
> @@ -1054,6 +1056,134 @@ flush-lines
>                              count))
>      count))
>  
> +(defun kill-lines (regexp &optional rstart rend interactive)
> +  "Kill lines containing matches for REGEXP.
> +
> +When called from Lisp (and usually when called interactively as
> +well, see below), applies to the part of the buffer after point.
> +The line point is in is killed if and only if it contains a match
> +for regexp starting after point.
       ^^^^^^
REGEXP should in all caps.

> +Second and third arg RSTART and REND specify the region to
                    ^^^
"args", in plural.

> +operate on.  Lines partially contained in this region are killed
> +if and only if they contain a match entirely contained in it.
                                                          ^^^^^
"in the region" will make this more clear.

> +                                                             When
> +calling this function from Lisp, you can pretend that it was
> +called interactively by passing a non-nil INTERACTIVE argument.

This is not specific to this command, so why tell it here?

Same comments apply to copy-lines.

> +(defun copy-lines (regexp &optional rstart rend interactive)
> +  "Copy lines containing matches for REGEXP to the kill ring.
> +
> +When called from Lisp (and usually when called interactively as
> +well, see below), applies to the part of the buffer after point.
> +The line point is in is copied if and only if it contains a match
> +for regexp starting after point.
> +
> +If REGEXP contains upper case characters (excluding those
> +preceded by `\\') and `search-upper-case' is non-nil, the
> +matching is case-sensitive.
> +
> +Second and third arg RSTART and REND specify the region to
> +operate on.  Lines partially contained in this region are copied
> +if and only if they contain a match entirely contained in it.
> +
> +Interactively, in Transient Mark mode when the mark is active,
> +operate on the contents of the region.  Otherwise, operate from
> +point to the end of (the accessible portion of) the buffer.  When
> +calling this function from Lisp, you can pretend that it was
> +called interactively by passing a non-nil INTERACTIVE argument.
> +
> +If a match is split across lines, all the lines it lies in are
> +copied.
> +
> +Return the number of copied matching lines.  When called
> +interactively, also print the number."
> +  (interactive
> +   (progn
> +     (barf-if-buffer-read-only)

Why barf? this command doesn't modify the buffer, does it?