GNU bug report logs - #73407
31.0.50; Add diff-discard-hunk

Previous Next

Package: emacs;

Reported by: Sean Whitton <spwhitton <at> spwhitton.name>

Date: Sat, 21 Sep 2024 10:20:01 UTC

Severity: normal

Found in version 31.0.50

Done: Sean Whitton <spwhitton <at> spwhitton.name>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to dgutov <at> yandex.ru, juri <at> linkov.net, bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Sat, 21 Sep 2024 10:20:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sean Whitton <spwhitton <at> spwhitton.name>:
New bug report received and forwarded. Copy sent to dgutov <at> yandex.ru, juri <at> linkov.net, bug-gnu-emacs <at> gnu.org. (Sat, 21 Sep 2024 10:20:01 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; Add diff-discard-hunk
Date: Sat, 21 Sep 2024 11:19:06 +0100
[Message part 1 (text/plain, inline)]
X-debbugs-cc: dgutov <at> yandex.ru, juri <at> linkov.net

These patches add a new command, diff-discard-hunk, inspired by some
functionality from Magit.  It nicely complements a workflow based around
committing with C-x v v from C-x v D.  I have been using it every day
for a year or so, and I think others will find it useful, too.

Posting for comments.  Thanks!

-- 
Sean Whitton
[0001-New-user-option-diff-display-after-apply-hunk.patch (text/x-patch, attachment)]
[0002-New-command-diff-discard-hunk.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Mon, 23 Sep 2024 22:53:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>, 73407 <at> debbugs.gnu.org
Cc: juri <at> linkov.net
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 01:52:13 +0300
Hi!

On 21/09/2024 13:19, Sean Whitton wrote:
> X-debbugs-cc: dgutov <at> yandex.ru, juri <at> linkov.net
> 
> These patches add a new command, diff-discard-hunk, inspired by some
> functionality from Magit.  It nicely complements a workflow based around
> committing with C-x v v from C-x v D.  I have been using it every day
> for a year or so, and I think others will find it useful, too.

I'd like to clarify one thing first: for 'C-x v v' to work correctly in 
a diff buffer (such as one produced by 'C-x v D') you don't have to 
synchronize any changes in that buffer back to disk before making the 
commit. In fact, we went to some effort to make this a non-requirement.

So when you just want to edit the patch before the commit, you can do so 
already, for example using 'M-k' (diff-hunk-kill) - or just 'k' if the 
buffer is read-only.

But of course if the idea is to really "discard" some changes, that works.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 06:55:03 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 73407 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 09:36:27 +0300
> These patches add a new command, diff-discard-hunk, inspired by some
> functionality from Magit.  It nicely complements a workflow based around
> committing with C-x v v from C-x v D.  I have been using it every day
> for a year or so, and I think others will find it useful, too.
>
> Posting for comments.  Thanks!

It was hard to understand how it's related to C-x v v.
But then I realized it saves time to do what currently is done with:
'C-c C-r' (diff-reverse-direction)
'C-c C-a' (diff-apply-hunk)
'k' (diff-hunk-kill).

> +(defcustom diff-display-after-apply-hunk t
> +  "Non-nil means `diff-apply-hunk' will display the buffer it just changed."
> +  :type 'boolean)
> +
> @@ -2024,7 +2028,8 @@ diff-apply-hunk
> -      (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
> +      (when diff-display-after-apply-hunk
> +        (set-window-point (display-buffer buf) (+ (car pos) (cdr new))))

Probably this change is not needed if 'diff-discard-hunk'
could avoid using 'diff-apply-hunk', and instead rely
on the algorithm from 'diff-apply-buffer'.  Also optional
region arguments 'beg' and 'end' could be added to
'diff-apply-buffer'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 07:54:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 08:53:10 +0100
Hello,

On Tue 24 Sep 2024 at 01:52am +03, Dmitry Gutov wrote:

> Hi!
>
> On 21/09/2024 13:19, Sean Whitton wrote:
>> X-debbugs-cc: dgutov <at> yandex.ru, juri <at> linkov.net
>> These patches add a new command, diff-discard-hunk, inspired by some
>> functionality from Magit.  It nicely complements a workflow based around
>> committing with C-x v v from C-x v D.  I have been using it every day
>> for a year or so, and I think others will find it useful, too.
>
> I'd like to clarify one thing first: for 'C-x v v' to work correctly in a diff
> buffer (such as one produced by 'C-x v D') you don't have to synchronize any
> changes in that buffer back to disk before making the commit. In fact, we went
> to some effort to make this a non-requirement.

Right.  I believe I was responsible for some of that effort!

> So when you just want to edit the patch before the commit, you can do so
> already, for example using 'M-k' (diff-hunk-kill) - or just 'k' if the buffer
> is read-only.
>
> But of course if the idea is to really "discard" some changes, that works.

Yeah, this is about really discarding the changes.

It's like using C-x v D as a kind of review view.  You use it for
committing what you want to keep and getting rid of temporary changes
you made just for development purposes, such as additional debug prints.

When I used Magit I used its magit-discard for this a lot (the docstring
for that function talks about conflicts but it's also useful without those).

One question I have is whether this should offer a y/n prompt to confirm
discarding the change.  Do you have an opinion on that?

-- 
Sean Whitton




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 08:09:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>
Cc: 73407 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 09:07:47 +0100
Hello,

On Tue 24 Sep 2024 at 09:36am +03, Juri Linkov wrote:

>> These patches add a new command, diff-discard-hunk, inspired by some
>> functionality from Magit.  It nicely complements a workflow based around
>> committing with C-x v v from C-x v D.  I have been using it every day
>> for a year or so, and I think others will find it useful, too.
>>
>> Posting for comments.  Thanks!
>
> It was hard to understand how it's related to C-x v v.
> But then I realized it saves time to do what currently is done with:
> 'C-c C-r' (diff-reverse-direction)
> 'C-c C-a' (diff-apply-hunk)
> 'k' (diff-hunk-kill).

Yeah, exactly.

>> +(defcustom diff-display-after-apply-hunk t
>> +  "Non-nil means `diff-apply-hunk' will display the buffer it just changed."
>> +  :type 'boolean)
>> +
>> @@ -2024,7 +2028,8 @@ diff-apply-hunk
>> -      (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
>> +      (when diff-display-after-apply-hunk
>> +        (set-window-point (display-buffer buf) (+ (car pos) (cdr new))))
>
> Probably this change is not needed if 'diff-discard-hunk'
> could avoid using 'diff-apply-hunk', and instead rely
> on the algorithm from 'diff-apply-buffer'.  Also optional
> region arguments 'beg' and 'end' could be added to
> 'diff-apply-buffer'.

I'll look into that, thanks.

-- 
Sean Whitton




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 08:42:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>, dgutov <at> yandex.ru
Cc: 73407 <at> debbugs.gnu.org
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 09:40:30 +0100
[Message part 1 (text/plain, inline)]
Hello,

Thank you both for looking.  Here is a revised patch based on Juri's
ideas about implementation, and also adding a y/n confirmation, since
this operation is destructive.

Let me know what you think!

-- 
Sean Whitton
[v2-0001-New-command-diff-discard-hunk.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 08:44:01 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>, dgutov <at> yandex.ru
Cc: 73407 <at> debbugs.gnu.org
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 09:42:49 +0100
[Message part 1 (text/plain, inline)]
Hello,

On Tue 24 Sep 2024 at 09:40am +01, Sean Whitton wrote:

> Hello,
>
> Thank you both for looking.  Here is a revised patch based on Juri's
> ideas about implementation, and also adding a y/n confirmation, since
> this operation is destructive.
>
> Let me know what you think!

I missed updating something in the info; here is v3.

-- 
Sean Whitton
[v3-0001-New-command-diff-discard-hunk.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 12:22:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 73407 <at> debbugs.gnu.org
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 15:20:52 +0300
On 24/09/2024 09:36, Juri Linkov wrote:
> 'C-c C-r' (diff-reverse-direction)
> 'C-c C-a' (diff-apply-hunk)

Also known as 'C-u C-c C-a'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 12:23:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: dgutov <at> yandex.ru, 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 15:21:42 +0300
> Cc: 73407 <at> debbugs.gnu.org
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Tue, 24 Sep 2024 09:40:30 +0100
> 
> +@findex diff-discard-hunk
> +@item C-c C-k
> +Reverse-apply this hunk to the target file, and then kill it
> +(@code{diff-discard-hunk}).  Unless the buffer visiting the target file
> +was already modified, save it.

We should come up with a better name for this command.
diff-discard-hunk tells me the command will discard the hunk, but says
nothing about applying it, let alone reverse-applying it.  How about
diff-revert-hunk, or maybe diff-revert-and-discard?

> +This command is useful in buffers generated by @w{@kbd{C-x v =}} and
> +@w{@kbd{C-x v D}} (@pxref{Old Revisions}).  You can use this command to
> +remove hunks you never intend to commit, such as temporary debug prints,
> +and the like.

This text could also use some clarification: "remove hunks you never
intended to commit" only hints on what it actually does.  A better
description would be something like "undo some of the changes you made
that you didn't intend".  I wouldn't mention "commit" at all, since
AFAIU this command is not specific to VC and doesn't require a VCS.

> -           (message "%d hunks failed; no buffers changed" failures)))))
> +           (message "%d hunks failed; no buffers changed" failures)
> +           failures))))

This comes from existing text, but still: what does the above say when
there's only 1 failure? does it say "1 hunks failed"?  If so, can we
improve the handling of singular/plural here?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 12:24:01 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>, Juri Linkov <juri <at> linkov.net>
Cc: 73407 <at> debbugs.gnu.org
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 15:22:56 +0300
On 24/09/2024 11:42, Sean Whitton wrote:
> +*** New command 'diff-discard-hunk' bound to C-c C-k.

I'm a little concerned about the binding.

We have 'diff-hunk-kill' on M-k (and also 'k'), and this new addition is 
actually more destructive. I'm expecting users will trip over the 
difference and call one of them when they wanted to call the other.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 12:26:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 15:23:57 +0300
On 24/09/2024 15:21, Eli Zaretskii wrote:

> We should come up with a better name for this command.
> diff-discard-hunk tells me the command will discard the hunk, but says
> nothing about applying it, let alone reverse-applying it.  How about
> diff-revert-hunk, or maybe diff-revert-and-discard?

diff-revert-and-kill, maybe.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 12:28:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 15:27:16 +0300
On 24/09/2024 10:53, Sean Whitton wrote:

>> I'd like to clarify one thing first: for 'C-x v v' to work correctly in a diff
>> buffer (such as one produced by 'C-x v D') you don't have to synchronize any
>> changes in that buffer back to disk before making the commit. In fact, we went
>> to some effort to make this a non-requirement.
> 
> Right.  I believe I was responsible for some of that effort!

Oh, right. Sorry.

>> So when you just want to edit the patch before the commit, you can do so
>> already, for example using 'M-k' (diff-hunk-kill) - or just 'k' if the buffer
>> is read-only.
>>
>> But of course if the idea is to really "discard" some changes, that works.
> 
> Yeah, this is about really discarding the changes.
> 
> It's like using C-x v D as a kind of review view.  You use it for
> committing what you want to keep and getting rid of temporary changes
> you made just for development purposes, such as additional debug prints.
> 
> When I used Magit I used its magit-discard for this a lot (the docstring
> for that function talks about conflicts but it's also useful without those).
> 
> One question I have is whether this should offer a y/n prompt to confirm
> discarding the change.  Do you have an opinion on that?

I guess we should? 'diff-hl-revert-hunk' also prompts, but it has a user 
option to disable that.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 14:34:01 GMT) Full text and rfc822 format available.

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

From: Óscar Fuentes <ofv <at> wanadoo.es>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, juri <at> linkov.net, 73407 <at> debbugs.gnu.org,
 Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 16:33:06 +0200
Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 24/09/2024 15:21, Eli Zaretskii wrote:
>
>> We should come up with a better name for this command.
>> diff-discard-hunk tells me the command will discard the hunk, but says
>> nothing about applying it, let alone reverse-applying it.  How about
>> diff-revert-hunk, or maybe diff-revert-and-discard?
>
> diff-revert-and-kill, maybe.

If this command works on hunks, please say it on the name:

diff-revert-and-kill-hunk

It makes the intent clear and greatly helps discoverability when using
certain tools that navigate the command names.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 15:45:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>, dgutov <at> yandex.ru
Cc: 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 16:43:48 +0100
[Message part 1 (text/plain, inline)]
Hello,

On Tue 24 Sep 2024 at 03:21pm +03, Eli Zaretskii wrote:

> We should come up with a better name for this command.
> diff-discard-hunk tells me the command will discard the hunk, but says
> nothing about applying it, let alone reverse-applying it.  How about
> diff-revert-hunk, or maybe diff-revert-and-discard?

Oh, nice.  I was just following Magit, but including "revert" is better,
indeed.  I think I prefer diff-revert-and-kill-hunk.

>> +This command is useful in buffers generated by @w{@kbd{C-x v =}} and
>> +@w{@kbd{C-x v D}} (@pxref{Old Revisions}).  You can use this command to
>> +remove hunks you never intend to commit, such as temporary debug prints,
>> +and the like.
>
> This text could also use some clarification: "remove hunks you never
> intended to commit" only hints on what it actually does.  A better
> description would be something like "undo some of the changes you made
> that you didn't intend".  I wouldn't mention "commit" at all, since
> AFAIU this command is not specific to VC and doesn't require a VCS.

Yes, this is the diff-mode manual, so that makes sense.
I've made a similar change in the docstring but kept a reference to
committing, there.

>> -           (message "%d hunks failed; no buffers changed" failures)))))
>> +           (message "%d hunks failed; no buffers changed" failures)
>> +           failures))))
>
> This comes from existing text, but still: what does the above say when
> there's only 1 failure? does it say "1 hunks failed"?  If so, can we
> improve the handling of singular/plural here?

I've installed a fix for that.

On Tue 24 Sep 2024 at 03:22pm +03, Dmitry Gutov wrote:

> I'm a little concerned about the binding.
>
> We have 'diff-hunk-kill' on M-k (and also 'k'), and this new addition is
> actually more destructive. I'm expecting users will trip over the difference
> and call one of them when they wanted to call the other.

Yeah, and thinking about it more, it also doesn't really match what C-c
C-k does elsewhere in Emacs.  I've made it C-c M-r in the attached v4.

-- 
Sean Whitton
[v4-0001-New-command-diff-revert-and-kill-hunk.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 15:49:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 16:48:25 +0100
Hello,

On Tue 24 Sep 2024 at 03:27pm +03, Dmitry Gutov wrote:

> Oh, right. Sorry.

Not at all!

>  'diff-hl-revert-hunk' also prompts, but it has a user option to
> disable that.

Hmm.  I've added prompting.  Let's try using it for a while and see if
we feel like such a user option is needed for this one too.

-- 
Sean Whitton




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 17:01:01 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 73407 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 19:59:04 +0300
>>> -           (message "%d hunks failed; no buffers changed" failures)))))
>>> +           (message "%d hunks failed; no buffers changed" failures)
>>> +           failures))))
>>
>> This comes from existing text, but still: what does the above say when
>> there's only 1 failure? does it say "1 hunks failed"?  If so, can we
>> improve the handling of singular/plural here?
>
> I've installed a fix for that.

Another variant would be to use 'ngettext'.

>> I'm a little concerned about the binding.
>>
>> We have 'diff-hunk-kill' on M-k (and also 'k'), and this new addition is
>> actually more destructive. I'm expecting users will trip over the difference
>> and call one of them when they wanted to call the other.
>
> Yeah, and thinking about it more, it also doesn't really match what C-c
> C-k does elsewhere in Emacs.  I've made it C-c M-r in the attached v4.

There is the existing 'C-c C-m' submap that could be used with e.g.
'C-c RET C-k'.


> +      (diff-reverse-direction beg end)
> +      (condition-case ret
> +          (diff-apply-buffer beg end)
> +        ;; Reversing the hunk is an implementation detail, so ensure the
> +        ;; user never sees it.
> +        (error (diff-reverse-direction beg end)
> +               (signal (car ret) (cdr ret)))
> +        (:success (if ret
> +                      (diff-reverse-direction beg end)

Instead of reversing the direction back and forth,
it would be simpler to add another optional argument
to 'diff-apply-buffer' to delegate it to the
REVERSE argument of 'diff-find-source-location'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 17:57:01 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 73407 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 18:56:07 +0100
Hello,

On Tue 24 Sep 2024 at 07:59pm +03, Juri Linkov wrote:

>>>> -           (message "%d hunks failed; no buffers changed" failures)))))
>>>> +           (message "%d hunks failed; no buffers changed" failures)
>>>> +           failures))))
>>>
>>> This comes from existing text, but still: what does the above say when
>>> there's only 1 failure? does it say "1 hunks failed"?  If so, can we
>>> improve the handling of singular/plural here?
>>
>> I've installed a fix for that.
>
> Another variant would be to use 'ngettext'.

TIL!

>>> I'm a little concerned about the binding.
>>>
>>> We have 'diff-hunk-kill' on M-k (and also 'k'), and this new addition is
>>> actually more destructive. I'm expecting users will trip over the difference
>>> and call one of them when they wanted to call the other.
>>
>> Yeah, and thinking about it more, it also doesn't really match what C-c
>> C-k does elsewhere in Emacs.  I've made it C-c M-r in the attached v4.
>
> There is the existing 'C-c C-m' submap that could be used with e.g.
> 'C-c RET C-k'.

I considered that, but I think I'd prefer that it's shorter sequence if
possible, because you might want to use it several times.  By contrast,
C-c C-m a is a command you probably just want once at a time.

>> +      (diff-reverse-direction beg end)
>> +      (condition-case ret
>> +          (diff-apply-buffer beg end)
>> +        ;; Reversing the hunk is an implementation detail, so ensure the
>> +        ;; user never sees it.
>> +        (error (diff-reverse-direction beg end)
>> +               (signal (car ret) (cdr ret)))
>> +        (:success (if ret
>> +                      (diff-reverse-direction beg end)
>
> Instead of reversing the direction back and forth,
> it would be simpler to add another optional argument
> to 'diff-apply-buffer' to delegate it to the
> REVERSE argument of 'diff-find-source-location'.

Thanks, I'll try that.

-- 
Sean Whitton




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Tue, 24 Sep 2024 19:11:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: dgutov <at> yandex.ru, 73407 <at> debbugs.gnu.org, spwhitton <at> spwhitton.name
Subject: Re: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Tue, 24 Sep 2024 21:07:42 +0300
> From: Juri Linkov <juri <at> linkov.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  dgutov <at> yandex.ru,  73407 <at> debbugs.gnu.org
> Date: Tue, 24 Sep 2024 19:59:04 +0300
> 
> >>> -           (message "%d hunks failed; no buffers changed" failures)))))
> >>> +           (message "%d hunks failed; no buffers changed" failures)
> >>> +           failures))))
> >>
> >> This comes from existing text, but still: what does the above say when
> >> there's only 1 failure? does it say "1 hunks failed"?  If so, can we
> >> improve the handling of singular/plural here?
> >
> > I've installed a fix for that.
> 
> Another variant would be to use 'ngettext'.

That's what I had in mind...




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Wed, 25 Sep 2024 06:38:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dgutov <at> yandex.ru, 73407 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Wed, 25 Sep 2024 07:36:42 +0100
Hello,

On Tue 24 Sep 2024 at 09:07pm +03, Eli Zaretskii wrote:

>> From: Juri Linkov <juri <at> linkov.net>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  dgutov <at> yandex.ru,  73407 <at> debbugs.gnu.org
>> Date: Tue, 24 Sep 2024 19:59:04 +0300
>>
>> >>> -           (message "%d hunks failed; no buffers changed" failures)))))
>> >>> +           (message "%d hunks failed; no buffers changed" failures)
>> >>> +           failures))))
>> >>
>> >> This comes from existing text, but still: what does the above say when
>> >> there's only 1 failure? does it say "1 hunks failed"?  If so, can we
>> >> improve the handling of singular/plural here?
>> >
>> > I've installed a fix for that.
>>
>> Another variant would be to use 'ngettext'.
>
> That's what I had in mind...

Do you mean like this?

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index be3d94db011..9677fbb9175 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2083,7 +2083,7 @@ diff-apply-buffer
            (message "Saved %d buffers" (length buffer-edits)))
           (t
            (message "%d %s failed; no buffers changed"
-                    failures (if (> failures 1) "hunks" "hunk"))))))
+                    failures (ngettext "hunk" "hunks" failures))))))

 (defalias 'diff-mouse-goto-source #'diff-goto-source)

-- 
Sean Whitton




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Wed, 25 Sep 2024 12:18:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: dgutov <at> yandex.ru, 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Wed, 25 Sep 2024 15:17:08 +0300
> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Cc: Juri Linkov <juri <at> linkov.net>,  dgutov <at> yandex.ru,  73407 <at> debbugs.gnu.org
> Date: Wed, 25 Sep 2024 07:36:42 +0100
> 
> >> Another variant would be to use 'ngettext'.
> >
> > That's what I had in mind...
> 
> Do you mean like this?

Yes.  But it is considered a better style not to break the message
into separate words, so the below would be even better:

  (message (ngettext "%d hunk failed; no buffers changed"
                     "%d hunks failed; no buffers changed"
                     failures))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73407; Package emacs. (Wed, 25 Sep 2024 19:35:02 GMT) Full text and rfc822 format available.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dgutov <at> yandex.ru, 73407 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk
Date: Wed, 25 Sep 2024 20:33:56 +0100
Hello,

On Wed 25 Sep 2024 at 03:17pm +03, Eli Zaretskii wrote:

> Yes.  But it is considered a better style not to break the message
> into separate words, so the below would be even better:
>
>   (message (ngettext "%d hunk failed; no buffers changed"
>                      "%d hunks failed; no buffers changed"
>                      failures))

Thanks -- installed.

-- 
Sean Whitton




Reply sent to Sean Whitton <spwhitton <at> spwhitton.name>:
You have taken responsibility. (Thu, 26 Sep 2024 10:54:02 GMT) Full text and rfc822 format available.

Notification sent to Sean Whitton <spwhitton <at> spwhitton.name>:
bug acknowledged by developer. (Thu, 26 Sep 2024 10:54:02 GMT) Full text and rfc822 format available.

Message #70 received at 73407-done <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: 73407-done <at> debbugs.gnu.org
Subject: Re: bug#73407: 31.0.50; Add diff-revert-and-kill-hunk
Date: Thu, 26 Sep 2024 11:52:30 +0100
Hello,

I believe all outstanding issues are resolved, so I've installed this, and am
closing the bug.  Thank you for the input, which improved my work, as ever.

-- 
Sean Whitton




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 24 Oct 2024 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 239 days ago.

Previous Next


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