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

[Okam]

On 6/19/21 1:33 PM, Eli Zaretskii wrote:
 >> 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.

Done.

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

I was following the examples of `flush-lines` and `keep-lines`, with
aliases `delete-matching-lines` and `delete-non-matching-lines`,
respectively. The proposed `kill-matching-lines` and
`copy-matching-lines` are just other versions of `flush-lines`.

I have removed the aliases from the patch.

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


Fixed.  Since the documentation string was originally copied from
`flush-lines`, do you want `keep-lines` and `flush-lines` to also have
these changes?

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

Fixed. Thank you.

Also, the `flush-lines` command, and so these proposed commands, reports
the number of, for example, deleted regions as the number of lines
removed, ignoring cases when the match spans multiple lines.  Should
that be changed too?

0001-Add-commands-kill-matching-lines-and-copy-matching-l.patch
Description: Text Data