GNU bug report logs - #65380
[PATCH] Add command to copy contents in a diff-mode buffer

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 65380 in the body.
You can then email your comments to 65380 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: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add command to copy contents in a diff-mode buffer
Date: Sat, 19 Aug 2023 09:53:58 +0000

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

Tags: patch


This command solves a long-standing annoyance I have had when working
with diffs.  To my knowledge there is no existing way to achieve this
(rectangular selection might be possible, but that behaves differently
when the text is yanked).  The binding "w" seems intuitive when
contrasted with "M-w", but might be confused with "W" (widen).  Not sure
if that is an issue or not.

In GNU Emacs 30.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.16.0) of 2023-08-18 built on quetzal
Repository revision: 2f74a459d2a82578fd9a92e9e2facdca90dad974
Repository branch: feature/a-package-for-elpa
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --with-pgtk --with-native-compilation --with-imagemagick
 --with-tree-sitter'

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/patch, attachment)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 10:00:49 +0000

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

Philip Kaludercic <philipk <at> posteo.net> writes:

> Tags: patch
>
>
> This command solves a long-standing annoyance I have had when working
> with diffs.  To my knowledge there is no existing way to achieve this
> (rectangular selection might be possible, but that behaves differently
> when the text is yanked).  The binding "w" seems intuitive when
> contrasted with "M-w", but might be confused with "W" (widen).  Not sure
> if that is an issue or not.
>
> In GNU Emacs 30.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version
>  3.24.37, cairo version 1.16.0) of 2023-08-18 built on quetzal
> Repository revision: 2f74a459d2a82578fd9a92e9e2facdca90dad974
> Repository branch: feature/a-package-for-elpa
> System Description: Debian GNU/Linux 12 (bookworm)
>
> Configured using:
>  'configure --with-pgtk --with-native-compilation --with-imagemagick
>  --with-tree-sitter'

It seems I prepared to patch too hastily, and forgot to add a docstring.
Here is the updated version, with a docstring:

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 13:46:01 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Sat, 19 Aug 2023 09:53:58 +0000
> 
> This command solves a long-standing annoyance I have had when working
> with diffs.  To my knowledge there is no existing way to achieve this
> (rectangular selection might be possible, but that behaves differently
> when the text is yanked).  The binding "w" seems intuitive when
> contrasted with "M-w", but might be confused with "W" (widen).  Not sure
> if that is an issue or not.

Please tell more about the use cases where this would be useful.  I
couldn't understand that from the patch itself.

Thanks.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 10:48:03 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Date: Sat, 19 Aug 2023 09:53:58 +0000
>> 
>> This command solves a long-standing annoyance I have had when working
>> with diffs.  To my knowledge there is no existing way to achieve this
>> (rectangular selection might be possible, but that behaves differently
>> when the text is yanked).  The binding "w" seems intuitive when
>> contrasted with "M-w", but might be confused with "W" (widen).  Not sure
>> if that is an issue or not.
>
> Please tell more about the use cases where this would be useful.  I
> couldn't understand that from the patch itself.

A simple example is where someone sends me a patch with a Elisp function
I'd like to evaluate.  If I can't or don't want to apply the patch right
now, I can use `diff-kill-ring-save' to copy the function without the
leading "+" into my scratch buffer and evaluate it there.

> Thanks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 14:06:55 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: 65380 <at> debbugs.gnu.org
> Date: Sat, 19 Aug 2023 10:48:03 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Please tell more about the use cases where this would be useful.  I
> > couldn't understand that from the patch itself.
> 
> A simple example is where someone sends me a patch with a Elisp function
> I'd like to evaluate.  If I can't or don't want to apply the patch right
> now, I can use `diff-kill-ring-save' to copy the function without the
> leading "+" into my scratch buffer and evaluate it there.

But diffs don't necessarily show the entire body/ies of function(s),
they show just part of them.  So this seems to be useful only in a
very small subset of cases?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 15:45:13 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: 65380 <at> debbugs.gnu.org
>> Date: Sat, 19 Aug 2023 10:48:03 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > Please tell more about the use cases where this would be useful.  I
>> > couldn't understand that from the patch itself.
>> 
>> A simple example is where someone sends me a patch with a Elisp function
>> I'd like to evaluate.  If I can't or don't want to apply the patch right
>> now, I can use `diff-kill-ring-save' to copy the function without the
>> leading "+" into my scratch buffer and evaluate it there.
>
> But diffs don't necessarily show the entire body/ies of function(s),
> they show just part of them.  So this seems to be useful only in a
> very small subset of cases?

In theory yes, but as I mentioned in my first message, it comes up
surprisingly often, at least to the degree that I think it would be
something nice to have in general.  If you think the current
implementation is too primitive, one could extend it to check if the
region is a subregion of a hunk, and handle that appropriately.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 22:09:12 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: 65380 <at> debbugs.gnu.org
> Date: Sat, 19 Aug 2023 15:45:13 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > But diffs don't necessarily show the entire body/ies of function(s),
> > they show just part of them.  So this seems to be useful only in a
> > very small subset of cases?
> 
> In theory yes, but as I mentioned in my first message, it comes up
> surprisingly often, at least to the degree that I think it would be
> something nice to have in general.  If you think the current
> implementation is too primitive, one could extend it to check if the
> region is a subregion of a hunk, and handle that appropriately.

I'd like to hear from more people who will find this useful enough to
have in Emacs.  My first thought was that this is something you should
keep as your local extension, but maybe I'm mistaken.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 19:30:27 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: 65380 <at> debbugs.gnu.org
>> Date: Sat, 19 Aug 2023 15:45:13 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > But diffs don't necessarily show the entire body/ies of function(s),
>> > they show just part of them.  So this seems to be useful only in a
>> > very small subset of cases?
>> 
>> In theory yes, but as I mentioned in my first message, it comes up
>> surprisingly often, at least to the degree that I think it would be
>> something nice to have in general.  If you think the current
>> implementation is too primitive, one could extend it to check if the
>> region is a subregion of a hunk, and handle that appropriately.
>
> I'd like to hear from more people who will find this useful enough to
> have in Emacs.  My first thought was that this is something you should
> keep as your local extension, but maybe I'm mistaken.

I have no problem with that, and there is no urgency in applying this
patch.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 19 Aug 2023 22:01:12 +0100

Hello,

On Sat 19 Aug 2023 at 10:09pm +03, Eli Zaretskii wrote:

> I'd like to hear from more people who will find this useful enough to
> have in Emacs.  My first thought was that this is something you should
> keep as your local extension, but maybe I'm mistaken.

This comes up for me in Debian packaging work, and I would like to have
this feature.  The diff contains the addition of a new test, say.  I
want to copy the test into a version of the package that's different
from the one the patch was originally prepared for.

-- 
Sean Whitton

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

From: Rudolf Adamkovič <salutis <at> me.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 00:49:15 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> I'd like to hear from more people who will find this useful enough to
> have in Emacs.  My first thought was that this is something you should
> keep as your local extension, but maybe I'm mistaken.

I, too, find myself removing those +/- signs quite often.

Rudy
-- 
"One can begin to reason only when a clear picture has been formed in
the imagination."
-- Walter Warwick Sawyer, Mathematician's Delight, 1943

Rudolf Adamkovič <salutis <at> me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

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

From: Daniel Martín <mardani29 <at> yahoo.es>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 02:41:04 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: 65380 <at> debbugs.gnu.org
>> Date: Sat, 19 Aug 2023 15:45:13 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > But diffs don't necessarily show the entire body/ies of function(s),
>> > they show just part of them.  So this seems to be useful only in a
>> > very small subset of cases?
>> 
>> In theory yes, but as I mentioned in my first message, it comes up
>> surprisingly often, at least to the degree that I think it would be
>> something nice to have in general.  If you think the current
>> implementation is too primitive, one could extend it to check if the
>> region is a subregion of a hunk, and handle that appropriately.
>
> I'd like to hear from more people who will find this useful enough to
> have in Emacs.  My first thought was that this is something you should
> keep as your local extension, but maybe I'm mistaken.

I'd find this command useful.  When I copy parts of a diff for other
purposes, I usually have to remove the diff markers manually.

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

From: Daniel Martín <mardani29 <at> yahoo.es>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 02:59:06 +0200

Philip Kaludercic <philipk <at> posteo.net> writes:

> +(defun diff-kill-ring-save (beg end)
> +  "Save contents of the region between BEG and END akin to `kill-ring-save'.
> +The contents of a region will not include diff indicators at the
> +beginning of each line."
> +  (interactive (list (region-beginning) (region-end)))
> +  (let ((at-bol (save-excursion (goto-char beg) (bolp)))
> +        lines)
> +    (save-restriction
> +      (narrow-to-region beg end)
> +      (goto-char (point-min))
> +      (while (not (eobp))
> +        (let ((line (thing-at-point 'line t)))
> +          ;; In case the user has selected a region that begins
> +          ;; mid-line, we should not chomp off the first character.
> +          (if (and (null lines) (not at-bol))
> +              (push line lines)
> +            (push (substring line 1) lines)))
> +        (forward-line)))
> +    (let ((region-extract-function
> +           (lambda (_) (apply #'concat (nreverse lines)))))
> +      (kill-ring-save beg end t))))
>

As an alternative implementation, to avoid creating lots of intermediate
substrings, we could check if we're inside a hunk and, if so, perform a
regular expression replace to remove the diff indicators.  If the region
covers text outside of a hunk, then we could copy the text normally, as
if the user pressed M-w.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 19:30:03 +0300

>> > But diffs don't necessarily show the entire body/ies of function(s),
>> > they show just part of them.  So this seems to be useful only in a
>> > very small subset of cases?
>>
>> In theory yes, but as I mentioned in my first message, it comes up
>> surprisingly often, at least to the degree that I think it would be
>> something nice to have in general.  If you think the current
>> implementation is too primitive, one could extend it to check if the
>> region is a subregion of a hunk, and handle that appropriately.
>
> I'd like to hear from more people who will find this useful enough to
> have in Emacs.  My first thought was that this is something you should
> keep as your local extension, but maybe I'm mistaken.

Sometimes it's useful to copy the plain text from the diff
without applying the patch.  Regarding the implementation, there is
the function 'diff-hunk-text', but it's limited to one hunk only.
So another variant would be to extend 'diff--filter-substring'
that conditionally could modify the kill-ring-save filter
depending on a new customizable variable.
Although for some strange reason, the filter is not applied
when using the region.  I think this is a bug, or at least
a misfeature.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 65380 <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 21:17:16 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Philip Kaludercic <philipk <at> posteo.net>,  65380 <at> debbugs.gnu.org
> Date: Sun, 20 Aug 2023 19:30:03 +0300
> 
> Regarding the implementation, there is the function
> 'diff-hunk-text', but it's limited to one hunk only.

So is the proposed new function, isn't it?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 18:24:53 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Juri Linkov <juri <at> linkov.net>
>> Cc: Philip Kaludercic <philipk <at> posteo.net>,  65380 <at> debbugs.gnu.org
>> Date: Sun, 20 Aug 2023 19:30:03 +0300
>> 
>> Regarding the implementation, there is the function
>> 'diff-hunk-text', but it's limited to one hunk only.
>
> So is the proposed new function, isn't it?

No, the current proposal doesn't have any special handling for text
between hunks.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 21:29:29 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: Juri Linkov <juri <at> linkov.net>,  65380 <at> debbugs.gnu.org
> Date: Sun, 20 Aug 2023 18:24:53 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Juri Linkov <juri <at> linkov.net>
> >> Cc: Philip Kaludercic <philipk <at> posteo.net>,  65380 <at> debbugs.gnu.org
> >> Date: Sun, 20 Aug 2023 19:30:03 +0300
> >> 
> >> Regarding the implementation, there is the function
> >> 'diff-hunk-text', but it's limited to one hunk only.
> >
> > So is the proposed new function, isn't it?
> 
> No, the current proposal doesn't have any special handling for text
> between hunks.

AFAIU, the function you proposed removes the first character from each
line in the region, so how will it handle multiple hunks?

Or maybe I misunderstood what you meant by "No"?

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 12:47:53 -0700

On 8/19/2023 12:09 PM, Eli Zaretskii wrote:
> I'd like to hear from more people who will find this useful enough to
> have in Emacs.  My first thought was that this is something you should
> keep as your local extension, but maybe I'm mistaken.

Based on my understanding of the current implementation, I would *not* 
find this useful, and instead I'd propose a couple of different ways to 
handle this.

First, the original message says, "rectangular selection might be 
possible, but that behaves differently when the text is yanked". I've 
been bitten by that a few times in the past, and I'd much rather a 
general solution to that problem instead. Some way of yanking a 
rectangular selection as though it were a normal region (or some way to 
put the rectangular selection on the normal kill ring) would be great, 
and would solve this in a more-general way. Then, for example, you could 
use the same command to copy the contents of a diff *or* to copy a 
commented-out block of code without the comment introducers. Something like:

  ;; (defun hello ()
  ;;   "Say hello."
  ;;   (message "Hello"))

If I could select a rectangle around the actual code to copy it, 
excluding the leading ";; ", that would be useful (occasionally, at least).

Second, for the diff case in particular, I'd rather have a command that 
copies the added or unchanged lines, and *skips* the removed lines. (As 
far as I can tell, the proposed implementation copies the removed lines 
as well.) Then I could actually yank this into my destination and it 
would work. Copying the removed lines would mean I need to go and remove 
them manually, only now I've lost the "-" indicator that tells me which 
lines are removed.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 20:13:53 +0000

>
> Some way of yanking a rectangular selection as though it were a normal 
> region (or some way to put the rectangular selection on the normal kill 
> ring) would be great, and would solve this in a more-general way.
>

These commands exist: C-x r k to kill a rectangular selection, C-x r M-w 
to save it, and C-x r y to yank it.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 13:45:20 -0700

On 8/20/2023 1:13 PM, Gregory Heytings wrote:
> 
>>
>> Some way of yanking a rectangular selection as though it were a normal 
>> region (or some way to put the rectangular selection on the normal 
>> kill ring) would be great, and would solve this in a more-general way.
>>
> 
> These commands exist: C-x r k to kill a rectangular selection, C-x r M-w 
> to save it, and C-x r y to yank it.

That's not quite what I mean. "C-x r y" ('yank-rectangle') yanks the 
rectangle *as a rectangle*. That is, if I just copied a rectangle with 5 
lines, and yank it with 'yank-rectangle', it will add it to the next 5 
existing lines. Instead, what I want is to insert 5 new lines.

For example, if my buffer is:

  Hello
  There
  World

And I copied this rectangle:

  1
  2

Then 'yank-rectangle' at the beginning of the buffer yields:

  1Hello
  2There
  World

But I want:

  1
  2
  Hello
  There
  World

The latter would be much more useful when trying to copy/yank a rect out 
of a diff buffer as in this report.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 21:29:43 +0000

>
> That's not quite what I mean. "C-x r y" ('yank-rectangle') yanks the 
> rectangle *as a rectangle*. That is, if I just copied a rectangle with 5 
> lines, and yank it with 'yank-rectangle', it will add it to the next 5 
> existing lines. Instead, what I want is to insert 5 new lines.
>

Indeed, I see what you mean.  Perhaps a new C-x r command that would do 
that could be added to Emacs?

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 15:21:30 -0700

On 8/20/2023 2:29 PM, Gregory Heytings wrote:
>
>> That's not quite what I mean. "C-x r y" ('yank-rectangle') yanks the 
>> rectangle *as a rectangle*. That is, if I just copied a rectangle with 
>> 5 lines, and yank it with 'yank-rectangle', it will add it to the next 
>> 5 existing lines. Instead, what I want is to insert 5 new lines.
>>
> 
> Indeed, I see what you mean.  Perhaps a new C-x r command that would do 
> that could be added to Emacs?

Yeah, the question then is: should it be new kill/copy commands or a new 
yank command? The former would mean you could use all the existing yank 
functions to paste the text in, but the latter means you can defer your 
decision about how to yank the text (as a regular region or as a 
rectangle) until you're ready to actually yank.

I'd lean a bit towards the former, but that does mean (potentially) two 
new key bindings.



... hmm, or maybe you could make the existing rectangle kill/copy 
commands also add to the "regular" kill ring automatically? But then 
that might cause issues with 'rectangle-mark-mode', where 'C-y' performs 
'yank-rectangle'[1]: how would I use 'rectangle-mark-mode' to copy a 
rect and then paste it as a regular region?

[1] Well, technically 'rectangle--insert-for-yank', but they both call 
'insert-rectangle' in the end.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 22:31:30 +0000

>
> Yeah, the question then is: should it be new kill/copy commands or a new 
> yank command?
>

I would do something else: a command to move the killed rectangle to the 
kill-ring.  And perhaps a command to do the opposite: transform the last 
item of the kill-ring into a killed rectangle.

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

From: Gregory Heytings <gregory <at> heytings.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 23:39:09 +0000

>> Yeah, the question then is: should it be new kill/copy commands or a 
>> new yank command?
>
> I would do something else: a command to move the killed rectangle to the 
> kill-ring.  And perhaps a command to do the opposite: transform the last 
> item of the kill-ring into a killed rectangle.
>

And here are the two commands I had in mind:

(defun move-killed-rectangle-to-kill-ring ()
  "Move the killed rectangle to the `kill-ring'."
  (interactive)
  (if killed-rectangle
      (with-temp-buffer
	(while killed-rectangle
	  (insert-for-yank (car killed-rectangle))
	  (insert "\n")
	  (setq killed-rectangle (cdr killed-rectangle)))
	(kill-ring-save (point-min) (point-max)))
    (user-error "No killed rectangle")))

(defun copy-last-kill-to-killed-rectangle ()
  "Transform the current item of `kill-ring' into a killed rectangle"
  (interactive)
  (with-temp-buffer
    (let ((max -1))
      (insert-for-yank (current-kill 0 t))
      (goto-char (point-min))
      (while (not (eobp))
	(move-end-of-line nil)
	(let ((col (current-column)))
	  (when (> col max)
	    (setq max col)))
	(forward-line 1))
      (let ((col (current-column)))
	(when (< col max)
	  (insert (make-string (- max col) ? ))))
    (kill-rectangle (point-min) (point-max)))))

WDYT?

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 17:34:43 -0700

On 8/20/2023 4:39 PM, Gregory Heytings wrote:
>> I would do something else: a command to move the killed rectangle to 
>> the kill-ring.  And perhaps a command to do the opposite: transform 
>> the last item of the kill-ring into a killed rectangle.
>>
> 
> And here are the two commands I had in mind:
[snip]> WDYT?

Hm, it could work. I'm not sure we *need* to be able to go from the kill 
ring to 'killed-rectangle', but if people are happy with the 
implementation, I don't see a problem.

I wonder how this would interact with 'rectangle-mark-mode' though. That 
puts the killed rectangle on the "regular" kill ring, but applies a 
'yank-handler' text property to the killed rect. 'rectangle-mark-mode' 
might need its own implementation for this. (Either not setting 
'yank-handler' when killing or having a way of ignoring the 
'yank-handler' when yanking?)

(I also wonder if it would make sense for the "normal" rect commands 
like "C-x r k" / 'kill-rectangle' to integrate with the kill ring like 
'rectangle-mark-mode', but that's another can of worms...)

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 22 Aug 2023 11:06:28 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: Juri Linkov <juri <at> linkov.net>,  65380 <at> debbugs.gnu.org
>> Date: Sun, 20 Aug 2023 18:24:53 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Juri Linkov <juri <at> linkov.net>
>> >> Cc: Philip Kaludercic <philipk <at> posteo.net>,  65380 <at> debbugs.gnu.org
>> >> Date: Sun, 20 Aug 2023 19:30:03 +0300
>> >> 
>> >> Regarding the implementation, there is the function
>> >> 'diff-hunk-text', but it's limited to one hunk only.
>> >
>> > So is the proposed new function, isn't it?
>> 
>> No, the current proposal doesn't have any special handling for text
>> between hunks.
>
> AFAIU, the function you proposed removes the first character from each
> line in the region, so how will it handle multiple hunks?
>
> Or maybe I misunderstood what you meant by "No"?

The command does not error when the region selects more than just a
sub-region of a hunk, but it doesn't do anything useful either.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 22 Aug 2023 14:29:34 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Tue, 22 Aug 2023 11:06:28 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk <at> posteo.net>
> >> Cc: Juri Linkov <juri <at> linkov.net>,  65380 <at> debbugs.gnu.org
> >> Date: Sun, 20 Aug 2023 18:24:53 +0000
> >> 
> >> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> 
> >> >> From: Juri Linkov <juri <at> linkov.net>
> >> >> Cc: Philip Kaludercic <philipk <at> posteo.net>,  65380 <at> debbugs.gnu.org
> >> >> Date: Sun, 20 Aug 2023 19:30:03 +0300
> >> >> 
> >> >> Regarding the implementation, there is the function
> >> >> 'diff-hunk-text', but it's limited to one hunk only.
> >> >
> >> > So is the proposed new function, isn't it?
> >> 
> >> No, the current proposal doesn't have any special handling for text
> >> between hunks.
> >
> > AFAIU, the function you proposed removes the first character from each
> > line in the region, so how will it handle multiple hunks?
> >
> > Or maybe I misunderstood what you meant by "No"?
> 
> The command does not error when the region selects more than just a
> sub-region of a hunk, but it doesn't do anything useful either.

Then how about making a command that is basically a wrapper around
diff-hunk-text, and allows to copy bodies of N hunks, given an
argument of N?  Wouldn't that be more useful?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Daniel Martín <mardani29 <at> yahoo.es>
Cc: 65380 <at> debbugs.gnu.org
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 20 Aug 2023 07:52:29 +0000

Daniel Martín <mardani29 <at> yahoo.es> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> +(defun diff-kill-ring-save (beg end)
>> +  "Save contents of the region between BEG and END akin to `kill-ring-save'.
>> +The contents of a region will not include diff indicators at the
>> +beginning of each line."
>> +  (interactive (list (region-beginning) (region-end)))
>> +  (let ((at-bol (save-excursion (goto-char beg) (bolp)))
>> +        lines)
>> +    (save-restriction
>> +      (narrow-to-region beg end)
>> +      (goto-char (point-min))
>> +      (while (not (eobp))
>> +        (let ((line (thing-at-point 'line t)))
>> +          ;; In case the user has selected a region that begins
>> +          ;; mid-line, we should not chomp off the first character.
>> +          (if (and (null lines) (not at-bol))
>> +              (push line lines)
>> +            (push (substring line 1) lines)))
>> +        (forward-line)))
>> +    (let ((region-extract-function
>> +           (lambda (_) (apply #'concat (nreverse lines)))))
>> +      (kill-ring-save beg end t))))
>>
>
> As an alternative implementation, to avoid creating lots of intermediate
> substrings, we could check if we're inside a hunk and, if so, perform a
> regular expression replace to remove the diff indicators.  If the region
> covers text outside of a hunk, then we could copy the text normally, as
> if the user pressed M-w.

What text would we want to copy outside of a hunk?  Something like a
patch commit message?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sat, 17 Aug 2024 16:34:17 +0000

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Tue, 22 Aug 2023 11:06:28 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Philip Kaludercic <philipk <at> posteo.net>
>> >> Cc: Juri Linkov <juri <at> linkov.net>,  65380 <at> debbugs.gnu.org
>> >> Date: Sun, 20 Aug 2023 18:24:53 +0000
>> >> 
>> >> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >> 
>> >> >> From: Juri Linkov <juri <at> linkov.net>
>> >> >> Cc: Philip Kaludercic <philipk <at> posteo.net>,  65380 <at> debbugs.gnu.org
>> >> >> Date: Sun, 20 Aug 2023 19:30:03 +0300
>> >> >> 
>> >> >> Regarding the implementation, there is the function
>> >> >> 'diff-hunk-text', but it's limited to one hunk only.
>> >> >
>> >> > So is the proposed new function, isn't it?
>> >> 
>> >> No, the current proposal doesn't have any special handling for text
>> >> between hunks.
>> >
>> > AFAIU, the function you proposed removes the first character from each
>> > line in the region, so how will it handle multiple hunks?
>> >
>> > Or maybe I misunderstood what you meant by "No"?
>> 
>> The command does not error when the region selects more than just a
>> sub-region of a hunk, but it doesn't do anything useful either.
>
> Then how about making a command that is basically a wrapper around
> diff-hunk-text, and allows to copy bodies of N hunks, given an
> argument of N?  Wouldn't that be more useful?

Sorry for the unreasonably delay in responding to this message.  I've
finally come up with something acceptable:

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-patch, attachment)]

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

This will also copy text between hunks.  I've tested it a bit, but I am
sure someone else here might find some edge-cases.

-- 
	Philip Kaludercic on peregrine

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 18 Aug 2024 15:29:25 +0000

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

Philip Kaludercic <philipk <at> posteo.net> writes:

> +          ;; copy the text between hunks
> +          (let (start)
> +            (save-window-excursion
> +              (save-excursion
> +                (forward-line -1)
> +                (diff-goto-source)
> +                (forward-line +1)
> +                (setq start (point))))

One issue I still have here is that the `forwarad-line' might scroll the
window, which `save-window-excursion' doesn't restore.  To fix this I
have written a general macro for subr.el to restore the scroll position:

[0002-Add-new-macro-to-restore-the-scroll-position-of-a-wi.patch (text/x-patch, attachment)]

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

and updated the previous patch accordingly (+ some other minor
improvements):

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-patch, attachment)]

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

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 18 Aug 2024 18:43:36 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Sun, 18 Aug 2024 15:29:25 +0000
> 
> > +          ;; copy the text between hunks
> > +          (let (start)
> > +            (save-window-excursion
> > +              (save-excursion
> > +                (forward-line -1)
> > +                (diff-goto-source)
> > +                (forward-line +1)
> > +                (setq start (point))))
> 
> One issue I still have here is that the `forwarad-line' might scroll the
> window, which `save-window-excursion' doesn't restore.

I don't understand what you are saying here.  As long as a Lisp
program runs and doesn't force redisplay (via sit-for or explicit
calls to 'redisplay' or somesuch), forwarad-line cannot cause any
scroll of the window, because what scrolls the window is redisplay
when it kicks in and finds that point is outside of the window.  So if
the above code takes care to restore point before it finishes, the
window won't scroll.  Or what am I missing?

> To fix this I have written a general macro for subr.el to restore
> the scroll position:

Let's first make sure we understand what happens here before we add
such a macro.  (It also has some conceptual problems of its own, but
let's defer that until we actually sure it is needed.)

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 18 Aug 2024 16:20:52 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Sun, 18 Aug 2024 15:29:25 +0000
>> 
>> > +          ;; copy the text between hunks
>> > +          (let (start)
>> > +            (save-window-excursion
>> > +              (save-excursion
>> > +                (forward-line -1)
>> > +                (diff-goto-source)
>> > +                (forward-line +1)
>> > +                (setq start (point))))
>> 
>> One issue I still have here is that the `forwarad-line' might scroll the
>> window, which `save-window-excursion' doesn't restore.
>
> I don't understand what you are saying here.  As long as a Lisp
> program runs and doesn't force redisplay (via sit-for or explicit
> calls to 'redisplay' or somesuch), forwarad-line cannot cause any
> scroll of the window, because what scrolls the window is redisplay
> when it kicks in and finds that point is outside of the window.  So if
> the above code takes care to restore point before it finishes, the
> window won't scroll.  Or what am I missing?

No, you were right, and I manged to locate what was causing the issue.
I had a (revert-buffer t t t) at the beginning of the function.
Wrapping `save-window-start' around just that expression also prevented
the displayed portion of the window from changing.

Thinking about the proposal again, it might seem better to avoid calling
`revert-buffer' in the first place.  The problem I was concerned with
was that if a .diff was outdated, jumping to the original file might not
do the right thing and copy the wrong inter-hunk text.  Now I realise
that `revert-buffer' doesn't always help resolve this either, e.g. if
copying text from an old .patch file.

The proper solution would be to try and detect these kinds of
inconsistencies and signal and error instead.

>> To fix this I have written a general macro for subr.el to restore
>> the scroll position:
>
> Let's first make sure we understand what happens here before we add
> such a macro.  (It also has some conceptual problems of its own, but
> let's defer that until we actually sure it is needed.)

It is not needed, but I would be interested in what the conceptual
errors are.

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Sun, 18 Aug 2024 21:00:19 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Sun, 18 Aug 2024 16:20:52 +0000
> 
> >> To fix this I have written a general macro for subr.el to restore
> >> the scroll position:
> >
> > Let's first make sure we understand what happens here before we add
> > such a macro.  (It also has some conceptual problems of its own, but
> > let's defer that until we actually sure it is needed.)
> 
> It is not needed, but I would be interested in what the conceptual
> errors are.

Forcing window-start has some side-effects that could look like bugs.
For example, the window's vscroll is reset, and if the original window
started its display with an overlay or display property,
set-window-start will not necessarily restore the original display,
due to boring technical issues.

So my recommendation is to avoid calling set-window-start as a means
to return to some previous display, because it is not guaranteed that
you can do it like that.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Mon, 19 Aug 2024 19:34:48 +0000

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Sun, 18 Aug 2024 16:20:52 +0000
>> 
>> >> To fix this I have written a general macro for subr.el to restore
>> >> the scroll position:
>> >
>> > Let's first make sure we understand what happens here before we add
>> > such a macro.  (It also has some conceptual problems of its own, but
>> > let's defer that until we actually sure it is needed.)
>> 
>> It is not needed, but I would be interested in what the conceptual
>> errors are.
>
> Forcing window-start has some side-effects that could look like bugs.
> For example, the window's vscroll is reset, and if the original window
> started its display with an overlay or display property,
> set-window-start will not necessarily restore the original display,
> due to boring technical issues.
>
> So my recommendation is to avoid calling set-window-start as a means
> to return to some previous display, because it is not guaranteed that
> you can do it like that.

OK, I understood and will keep it in mind for the future.  Thank you for
the background!

I'm attaching the newest version of the patch here:

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-patch, attachment)]

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

-- 
	Philip Kaludercic on peregrine

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

From: Juri Linkov <juri <at> linkov.net>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 09:44:22 +0300

> I'm attaching the newest version of the patch here:

Thanks, I tried it out, and it works nicely for the single hunk case.

> +*** New command 'diff-kill-ring-save'
> +This command copies out the modified contents out of a diff, without
> +having to apply it first.  If the selected range extends a hunk, the
> +commands attempts to look up and copy the text between from the
> +referenced file.

I'm not sure about usefulness of the last part in multi-hunk case.
Does someone really need to copy a huge part of the source file
between hunks at the top and bottom?  I expected that multi-hunk case
would copy only concatenated text of hunks, not the source file.
But I have no strong opinion about this.  My main use case will be
to copy the text of the current hunk.  Could you please support
this case where typing 'w' would copy the current hunk when the
region is not activated.

> @@ -630,6 +631,22 @@ diff-end-of-hunk
> +(defun diff-beginning-of-hunk-position (&optional try-harder)
> +  "Call `diff-end-of-hunk' without moving.
> +The optional argument TRY-HARDER is passed on to
> +`diff-beginning-of-hunk'."
> +  (save-excursion
> +    (save-window-excursion
> +      (diff-beginning-of-hunk try-harder))))
> +
> +(defun diff-end-of-hunk-position (&optional style donttrustheader)
> +  "Call `diff-end-of-hunk' without moving.
> +The optional arguments STYLE and DONTTRUSTHEADER are passed on to
> +`diff-end-of-hunk'."
> +  (save-excursion
> +    (save-window-excursion
> +      (diff-end-of-hunk style donttrustheader))))

I don't understand why separate functions with 'save-window-excursion'
are needed here, since all other uses of 'diff-beginning-of-hunk'
just wrap the calls with 'save-excursion'.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Juri Linkov <juri <at> linkov.net>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 07:46:43 +0000

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

Juri Linkov <juri <at> linkov.net> writes:

>> I'm attaching the newest version of the patch here:
>
> Thanks, I tried it out, and it works nicely for the single hunk case.

1+

>> +*** New command 'diff-kill-ring-save'
>> +This command copies out the modified contents out of a diff, without
>> +having to apply it first.  If the selected range extends a hunk, the
>> +commands attempts to look up and copy the text between from the
>> +referenced file.
>
> I'm not sure about usefulness of the last part in multi-hunk case.
> Does someone really need to copy a huge part of the source file
> between hunks at the top and bottom?  I expected that multi-hunk case
> would copy only concatenated text of hunks, not the source file.
> But I have no strong opinion about this.  

The main scenario I had in mind was when someone modifies the beginning
and the end of a function, and I want to copy all of it at once.  I
agree that there is usually not much of a use for copying huge parts of
a source file, but we get that for free when the beginning-to-end of a
function case is handled.

>                                           My main use case will be
> to copy the text of the current hunk.  Could you please support
> this case where typing 'w' would copy the current hunk when the
> region is not activated.

Done, see below.

>> @@ -630,6 +631,22 @@ diff-end-of-hunk
>> +(defun diff-beginning-of-hunk-position (&optional try-harder)
>> +  "Call `diff-end-of-hunk' without moving.
>> +The optional argument TRY-HARDER is passed on to
>> +`diff-beginning-of-hunk'."
>> +  (save-excursion
>> +    (save-window-excursion
>> +      (diff-beginning-of-hunk try-harder))))
>> +
>> +(defun diff-end-of-hunk-position (&optional style donttrustheader)
>> +  "Call `diff-end-of-hunk' without moving.
>> +The optional arguments STYLE and DONTTRUSTHEADER are passed on to
>> +`diff-end-of-hunk'."
>> +  (save-excursion
>> +    (save-window-excursion
>> +      (diff-end-of-hunk style donttrustheader))))
>
> I don't understand why separate functions with 'save-window-excursion'
> are needed here, since all other uses of 'diff-beginning-of-hunk'
> just wrap the calls with 'save-excursion'.

You are right, it isn't needed anymore, and I have removed it.

Here is the updated patch:

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-patch, attachment)]

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

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 14:36:58 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Mon, 19 Aug 2024 19:34:48 +0000
> 
> +*** New command 'diff-kill-ring-save'
> +This command copies out the modified contents out of a diff, without
> +having to apply it first.

This could be reworded to make the effect of the command more clear.
For example:

  This command copies to the 'kill-ring' a region of text modified
  according to diffs in the current buffer, but without applying the
  diffs to the original text.

>                            If the selected range extends a hunk, the
> +commands attempts to look up and copy the text between from the
> +referenced file.                          ^^^^^^^^^^^^^^^^^

Something is missing (or redundant) in this sentence.

> +(defun diff-kill-ring-save (beg end &optional reverse)
> +  "Save contents of the region between BEG and END akin to `kill-ring-save'.

The first line should IMO say something to make it clear this command
uses diffs from the current buffer.  OTOH, the reference to
`kill-ring-save' is much less important.  So how about

  Save to `kill-ring' the result of applying diffs in region between BEG and END.

> +By default the command will copy the text that applying the diff would
> +produce, along with the text between hunks.  If REVERSE is non-nil, or
> +the command was invoked with a prefix argument, copy the deleted text."

The "deleted text" part here is unclear: who or what deletes text and
what text is deleted?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 12:10:01 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Mon, 19 Aug 2024 19:34:48 +0000
>> 
>> +*** New command 'diff-kill-ring-save'
>> +This command copies out the modified contents out of a diff, without
>> +having to apply it first.
>
> This could be reworded to make the effect of the command more clear.
> For example:
>
>   This command copies to the 'kill-ring' a region of text modified
>   according to diffs in the current buffer, but without applying the
>   diffs to the original text.

Applied

>>                            If the selected range extends a hunk, the
>> +commands attempts to look up and copy the text between from the
>> +referenced file.                          ^^^^^^^^^^^^^^^^^
>
> Something is missing (or redundant) in this sentence.

How about "the text in between from".  Does that sound better?
                    ^^

>> +(defun diff-kill-ring-save (beg end &optional reverse)
>> +  "Save contents of the region between BEG and END akin to `kill-ring-save'.
>
> The first line should IMO say something to make it clear this command
> uses diffs from the current buffer.  OTOH, the reference to
> `kill-ring-save' is much less important.  So how about
>
>   Save to `kill-ring' the result of applying diffs in region between BEG and END.

I like it.

>> +By default the command will copy the text that applying the diff would
>> +produce, along with the text between hunks.  If REVERSE is non-nil, or
>> +the command was invoked with a prefix argument, copy the deleted text."
>
> The "deleted text" part here is unclear: who or what deletes text and
> what text is deleted?

I want to express that it copies the parts of the diff, that the
changeset removes (lines beginning with "-").  So perhaps "... copy the text
removed in the diff" would be better?

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 16:09:00 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Tue, 20 Aug 2024 12:10:01 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >>                            If the selected range extends a hunk, the
> >> +commands attempts to look up and copy the text between from the
> >> +referenced file.                          ^^^^^^^^^^^^^^^^^
> >
> > Something is missing (or redundant) in this sentence.
> 
> How about "the text in between from".  Does that sound better?
>                     ^^

If that's what you mean, I suggest "the text in-between the hunks".

> >> +By default the command will copy the text that applying the diff would
> >> +produce, along with the text between hunks.  If REVERSE is non-nil, or
> >> +the command was invoked with a prefix argument, copy the deleted text."
> >
> > The "deleted text" part here is unclear: who or what deletes text and
> > what text is deleted?
> 
> I want to express that it copies the parts of the diff, that the
> changeset removes (lines beginning with "-").  So perhaps "... copy the text
> removed in the diff" would be better?

I think "copy the text removed by the diffs", and perhaps say
explicitly that those are the lines in the hunks preceded with "-".

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 16:23:09 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Tue, 20 Aug 2024 12:10:01 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >>                            If the selected range extends a hunk, the
>> >> +commands attempts to look up and copy the text between from the
>> >> +referenced file.                          ^^^^^^^^^^^^^^^^^
>> >
>> > Something is missing (or redundant) in this sentence.
>> 
>> How about "the text in between from".  Does that sound better?
>>                     ^^
>
> If that's what you mean, I suggest "the text in-between the hunks".

Stole that idea :)

>> >> +By default the command will copy the text that applying the diff would
>> >> +produce, along with the text between hunks.  If REVERSE is non-nil, or
>> >> +the command was invoked with a prefix argument, copy the deleted text."
>> >
>> > The "deleted text" part here is unclear: who or what deletes text and
>> > what text is deleted?
>> 
>> I want to express that it copies the parts of the diff, that the
>> changeset removes (lines beginning with "-").  So perhaps "... copy the text
>> removed in the diff" would be better?
>
> I think "copy the text removed by the diffs", and perhaps say
> explicitly that those are the lines in the hunks preceded with "-".

According to `diff-font-lock-keywords' diff-mode also supports the "<"
syntax for removed parts of a hunk (I believe this comes from RCS?).
Then again, I would assume that the structure and contents of a diff
should be familiar to anyone using diff-mode seriously enough to be
interested in a command like this one?

-- 
	Philip Kaludercic on peregrine

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

From: Juri Linkov <juri <at> linkov.net>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 19:53:27 +0300

> The main scenario I had in mind was when someone modifies the beginning
> and the end of a function, and I want to copy all of it at once.  I
> agree that there is usually not much of a use for copying huge parts of
> a source file, but we get that for free when the beginning-to-end of a
> function case is handled.

I see, this is useful for the group of adjacent related hunks.

>>                                           My main use case will be
>> to copy the text of the current hunk.  Could you please support
>> this case where typing 'w' would copy the current hunk when the
>> region is not activated.
>
> Done, see below.

Thank you!

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 21:43:00 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Tue, 20 Aug 2024 16:23:09 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I think "copy the text removed by the diffs", and perhaps say
> > explicitly that those are the lines in the hunks preceded with "-".
> 
> According to `diff-font-lock-keywords' diff-mode also supports the "<"
> syntax for removed parts of a hunk (I believe this comes from RCS?).

No, that's the original Diff format, before we had -c and -u.

By all means mention the "<" lines as well.

> Then again, I would assume that the structure and contents of a diff
> should be familiar to anyone using diff-mode seriously enough to be
> interested in a command like this one?

Yes, but the connection between that and what you mean by "deleted
text" might not be obvious to everyone.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Tue, 20 Aug 2024 21:35:06 +0000

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Tue, 20 Aug 2024 16:23:09 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > I think "copy the text removed by the diffs", and perhaps say
>> > explicitly that those are the lines in the hunks preceded with "-".
>> 
>> According to `diff-font-lock-keywords' diff-mode also supports the "<"
>> syntax for removed parts of a hunk (I believe this comes from RCS?).
>
> No, that's the original Diff format, before we had -c and -u.
>
> By all means mention the "<" lines as well.
>
>> Then again, I would assume that the structure and contents of a diff
>> should be familiar to anyone using diff-mode seriously enough to be
>> interested in a command like this one?
>
> Yes, but the connection between that and what you mean by "deleted
> text" might not be obvious to everyone.

OK on both counts, here is the updated version:

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-patch, attachment)]

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

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Wed, 21 Aug 2024 16:42:40 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Tue, 20 Aug 2024 21:35:06 +0000
> 
> > Yes, but the connection between that and what you mean by "deleted
> > text" might not be obvious to everyone.
> 
> OK on both counts, here is the updated version:

Thanks.

> +*** New command 'diff-kill-ring-save'
> +This command copies out the modified contents out of a diff, without
                       ^^^                       ^^^
One of these two "outs"s should be removed, I think.

> +having to apply it first.  If the selected range extends a hunk, the
> +commands attempts to look up and copy the text between from the
> +referenced file.

I think it is better to say "copies to the kill-ring the modified
contents..."

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Wed, 21 Aug 2024 19:40:01 +0000

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Tue, 20 Aug 2024 21:35:06 +0000
>> 
>> > Yes, but the connection between that and what you mean by "deleted
>> > text" might not be obvious to everyone.
>> 
>> OK on both counts, here is the updated version:
>
> Thanks.
>
>> +*** New command 'diff-kill-ring-save'
>> +This command copies out the modified contents out of a diff, without
>                        ^^^                       ^^^
> One of these two "outs"s should be removed, I think.
>
>> +having to apply it first.  If the selected range extends a hunk, the
>> +commands attempts to look up and copy the text between from the
>> +referenced file.
>
> I think it is better to say "copies to the kill-ring the modified
> contents..."

Ugh, I forgot to amend by last patch, this is the current version:

  This command copies to the 'kill-ring' a region of text modified
  according to diffs in the current buffer, but without applying the
  diffs to the original text.  If the selected range extends a hunk, the
  commands attempts to look up and copy the text in-between the
  hunks.

I am thinking about splitting the first sentence up into

  This command copies text out of a diff to the 'kill-ring'.  By default
  it will use the text the diff would modify, without having to apply a
  hunk.  If the selected range extends a hunk, the commands attempts to
  look up and copy the text in-between the hunks.

WDYT?

Here is the patch as it is:

[0001-Add-command-to-copy-contents-in-a-diff-mode-buffer.patch (text/x-patch, inline)]

From 494b9ea69a501f3420c8eee1080d7d45637e39d5 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 19 Aug 2023 11:47:54 +0200
Subject: [PATCH] Add command to copy contents in a diff-mode buffer

* lisp/vc/diff-mode.el (diff-mode-shared-map): Bind 'diff-kill-ring-save'.
(diff-mode-map): Ensure the "w" binding does not get prefixed.
(diff-kill-ring-save): Add the command.
* etc/NEWS: Mention 'diff-kill-ring-save'.  (Bug#65380)
---
 etc/NEWS             | 10 +++++++++
 lisp/vc/diff-mode.el | 52 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index b89a80aa14d..25ceb408e2c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -210,6 +210,16 @@ The host name for Kubernetes connections can be of kind
 used.  This overrides the setiing in 'tramp-kubernetes-namespace', if
 any.
 
+** Diff
+
+---
+*** New command 'diff-kill-ring-save'
+This command copies to the 'kill-ring' a region of text modified
+according to diffs in the current buffer, but without applying the
+diffs to the original text.  If the selected range extends a hunk, the
+commands attempts to look up and copy the text in-between the
+hunks.
+
 
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 81e8b23ee33..4810b9ce01c 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -196,6 +196,7 @@ diff-mode-shared-map
   "RET" #'diff-goto-source
   "<mouse-2>" #'diff-goto-source
   "W" #'widen
+  "w" #'diff-kill-ring-save
   "o" #'diff-goto-source                ; other-window
   "A" #'diff-ediff-patch
   "r" #'diff-restrict-view
@@ -208,7 +209,7 @@ diff-mode-map
           ;; We want to inherit most bindings from
           ;; `diff-mode-shared-map', but not all since they may hide
           ;; useful `M-<foo>' global bindings when editing.
-          (dolist (key '("A" "r" "R" "g" "q" "W" "z"))
+          (dolist (key '("A" "r" "R" "g" "q" "W" "w" "z"))
             (keymap-set map key nil))
           map)
   ;; From compilation-minor-mode.
@@ -2108,6 +2109,55 @@ diff-goto-source
       (goto-char (+ (car pos) (cdr src)))
       (when buffer (next-error-found buffer (current-buffer))))))
 
+(defun diff-kill-ring-save (beg end &optional reverse)
+  "Save to `kill-ring' the result of applying diffs in region between BEG and END.
+By default the command will copy the text that applying the diff would
+produce, along with the text between hunks.  If REVERSE is non-nil, or
+the command was invoked with a prefix argument, copy the lines that the
+diff would remove (beginning with \"+\" or \"<\")."
+  (interactive
+   (append (if (use-region-p)
+               (list (region-beginning) (region-end))
+             (save-excursion
+               (list (diff-beginning-of-hunk)
+                     (diff-end-of-hunk))))
+           (list current-prefix-arg)))
+  (unless (derived-mode-p 'diff-mode)
+    (user-error "Command can only be invoked in a diff-buffer"))
+  (let ((parts '()))
+    (save-excursion
+      (goto-char beg)
+      (catch 'break
+        (while t
+          (let ((hunk (diff-hunk-text
+                       (buffer-substring
+                        (save-excursion (diff-beginning-of-hunk))
+                        (save-excursion (min (diff-end-of-hunk) end)))
+                       (not reverse)
+                       (save-excursion
+                         (- (point) (diff-beginning-of-hunk))))))
+            (push (substring (car hunk) (cdr hunk))
+                  parts))
+          ;; check if we have copied everything
+          (diff-end-of-hunk)
+          (when (<= end (point)) (throw 'break t))
+          ;; copy the text between hunks
+          (let ((inhibit-message t) start)
+            (save-window-excursion
+              (save-excursion
+                (forward-line -1)
+                ;; FIXME: Detect if the line we jump to doesn't match
+                ;; the line in the diff.
+                (diff-goto-source t)
+                (forward-line +1)
+                (setq start (point))))
+            (save-window-excursion
+              (diff-goto-source t)
+              (push (buffer-substring start (point))
+                    parts))))))
+    (kill-new (string-join (nreverse parts)))
+    (setq deactivate-mark t)
+    (message (if reverse "Copied original text" "Copied modified text"))))
 
 (defun diff-current-defun ()
   "Find the name of function at point.
-- 
2.46.0

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

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Thu, 22 Aug 2024 06:25:55 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Wed, 21 Aug 2024 19:40:01 +0000
> 
> Ugh, I forgot to amend by last patch, this is the current version:
> 
>   This command copies to the 'kill-ring' a region of text modified
>   according to diffs in the current buffer, but without applying the
>   diffs to the original text.  If the selected range extends a hunk, the
>   commands attempts to look up and copy the text in-between the
>   hunks.
> 
> I am thinking about splitting the first sentence up into
> 
>   This command copies text out of a diff to the 'kill-ring'.  By default
>   it will use the text the diff would modify, without having to apply a
>   hunk.  If the selected range extends a hunk, the commands attempts to
>   look up and copy the text in-between the hunks.
> 
> WDYT?

Two comments:

 . "copies text out of a diff" is hard to understand.  My
   understanding is that it takes the original text, modifies it using
   the diff hunks in the region, and copies the result to kill-ring.
   That's what I tried to describe in the first sentence that you now
   want to split.  If that interpretation is correct, then "copies
   text out of a diff" makes it much less clear.
 . The split version says "by default", but doesn't clearly say what
   will happen "not by default".  If the next sentence is that
   "non-default" case, then it is better to join these two sentences:
   "By default, ..., but if the selected text extends a hunk, ...".
   And in that latter case, I'm not sure I understand the significance
   of "by default", to tell the truth.

Or maybe I simply misunderstand what the patch does, as I'm not an
expert on diff-mode and don't know well enough what the various
functions you call do.  But then neither will the prospective reader
of this NEWS entry.

Thanks.

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Thu, 22 Aug 2024 06:41:22 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Wed, 21 Aug 2024 19:40:01 +0000
>> 
>> Ugh, I forgot to amend by last patch, this is the current version:
>> 
>>   This command copies to the 'kill-ring' a region of text modified
>>   according to diffs in the current buffer, but without applying the
>>   diffs to the original text.  If the selected range extends a hunk, the
>>   commands attempts to look up and copy the text in-between the
>>   hunks.
>> 
>> I am thinking about splitting the first sentence up into
>> 
>>   This command copies text out of a diff to the 'kill-ring'.  By default
>>   it will use the text the diff would modify, without having to apply a
>>   hunk.  If the selected range extends a hunk, the commands attempts to
>>   look up and copy the text in-between the hunks.
>> 
>> WDYT?
>
> Two comments:
>
>  . "copies text out of a diff" is hard to understand.  My
>    understanding is that it takes the original text, modifies it using
>    the diff hunks in the region, and copies the result to kill-ring.
>    That's what I tried to describe in the first sentence that you now
>    want to split.  If that interpretation is correct, then "copies
>    text out of a diff" makes it much less clear.
>  . The split version says "by default", but doesn't clearly say what
>    will happen "not by default".  If the next sentence is that
>    "non-default" case, then it is better to join these two sentences:
>    "By default, ..., but if the selected text extends a hunk, ...".
>    And in that latter case, I'm not sure I understand the significance
>    of "by default", to tell the truth.
>
> Or maybe I simply misunderstand what the patch does, as I'm not an
> expert on diff-mode and don't know well enough what the various
> functions you call do.  But then neither will the prospective reader
> of this NEWS entry.

No, your understanding was ring.  I'll go ahead and use your suggestion
then.

Is there anything else left to discuss?

> Thanks.

-- 
	Philip Kaludercic on peregrine

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65380 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Thu, 22 Aug 2024 13:22:26 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
> Date: Thu, 22 Aug 2024 06:41:22 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Or maybe I simply misunderstand what the patch does, as I'm not an
> > expert on diff-mode and don't know well enough what the various
> > functions you call do.  But then neither will the prospective reader
> > of this NEWS entry.
> 
> No, your understanding was ring.  I'll go ahead and use your suggestion
> then.
> 
> Is there anything else left to discuss?

Not from me, no.

Message #148 received at 65380-close <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65380-close <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode
 buffer
Date: Thu, 22 Aug 2024 18:59:31 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  65380 <at> debbugs.gnu.org
>> Date: Thu, 22 Aug 2024 06:41:22 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > Or maybe I simply misunderstand what the patch does, as I'm not an
>> > expert on diff-mode and don't know well enough what the various
>> > functions you call do.  But then neither will the prospective reader
>> > of this NEWS entry.
>> 
>> No, your understanding was ring.  I'll go ahead and use your suggestion
>> then.
>> 
>> Is there anything else left to discuss?
>
> Not from me, no.

OK, I have pushed the patch to master and am closing this report.

-- 
	Philip Kaludercic on peregrine

Previous Next

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