GNU bug report logs - #49120
[PATCH] Add commands 'kill-lines' and 'copy-lines'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49120 in the body.
You can then email your comments to 49120 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Okam <okamsn <at> protonmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add commands 'kill-lines' and 'copy-lines'
Date: Sat, 19 Jun 2021 17:12:25 +0000

[Message part 1 (text/plain, inline)]

These commands work similarly to the 'flush-lines' command, but add the
lines to the kill ring as a single item.

For example, when working with text lists, such as in

     - Documentation: Do thing 1
     - Source: Add new function
     - Documentation: Do thing 2

if one wanted to copy only the lines containing "Documentation", while
leaving the buffer untouched, the 'copy-lines' command could be used.

[0001-Add-commands-kill-lines-and-copy-lines.patch (text/x-patch, attachment)]

Message #8 received at 49120 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Okam <okamsn <at> protonmail.com>
Cc: 49120 <at> debbugs.gnu.org
Subject: Re: bug#49120: [PATCH] Add commands 'kill-lines' and 'copy-lines'
Date: Sat, 19 Jun 2021 20:33:38 +0300

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

Message #11 received at 49120 <at> debbugs.gnu.org (full text, mbox):

From: Okam <okamsn <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 49120 <at> debbugs.gnu.org
Subject: Re: bug#49120: [PATCH] Add commands 'kill-lines' and 'copy-lines'
Date: Thu, 01 Jul 2021 23:50:26 +0000

[Message part 1 (text/plain, inline)]

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 <at> 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 (text/x-patch, attachment)]

Message #14 received at 49120 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Okam <okamsn <at> protonmail.com>
Cc: 49120 <at> debbugs.gnu.org
Subject: Re: bug#49120: [PATCH] Add commands 'kill-lines' and 'copy-lines'
Date: Sat, 03 Jul 2021 10:07:00 +0300

> Date: Thu, 01 Jul 2021 23:50:26 +0000
> From: Okam <okamsn <at> protonmail.com>
> Cc: 49120 <at> debbugs.gnu.org
> 
>  >> +(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?

Yes.  But that should be a separate patch.

The patch LGTM, thanks.

Message #17 received at 49120 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Okam <okamsn <at> protonmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 49120 <at> debbugs.gnu.org
Subject: Re: bug#49120: [PATCH] Add commands 'kill-lines' and 'copy-lines'
Date: Tue, 20 Jul 2021 14:10:59 +0200

Okam <okamsn <at> protonmail.com> writes:

> * doc/emacs/search.texi: Document these additions.
> * lisp/replace.el:
> Add the commands 'kill-matching-lines' and 'copy-matching-lines'.

Thanks; added to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.