Re: master c86e7a29e78: ; Fix documentation of last change

Re: master c86e7a29e78: ; Fix documentation of last change

[Sean Whitton]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

Hello,

On Thu, Sep 26, 2024 at 07:25:03AM -0400, Eli Zaretskii wrote:
> --- a/doc/emacs/files.texi
> +++ b/doc/emacs/files.texi
> @@ -1675,6 +1675,7 @@ In a multi-file patch, kill the current file part.
>  
>  @findex diff-apply-hunk
>  @cindex patches, applying
> +@cindex reverse-apply diffs
>  @item C-c C-a
>  Apply this hunk to its target file (@code{diff-apply-hunk}).  With a
>  prefix argument of @kbd{C-u}, revert this hunk, i.e.@: apply the

Nice.

> diff --git a/etc/NEWS b/etc/NEWS
> index feadc54c17a..cdc7f47b7a9 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -365,8 +365,9 @@ command attempts to look up and copy the text in-between the hunks.
>  
>  +++
>  *** New command 'diff-revert-and-kill-hunk' bound to C-c M-r.
> -This command reverse-applies the hunk at point, and then kills it.
> -This is useful in buffers generated by C-x v = and C-x v D.
> +This command reverts the hunk at point (i.e., applies the reverse of the
> +hunk), and then removes the hunk from the diffs.  This is useful to undo
> +commits when you are in buffers generated by 'C-x v =' and 'C-x v D'.

   ^^^^^^^
   I think this should be "changes", right?

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: master c86e7a29e78: ; Fix documentation of last change

[Eli Zaretskii]

> Date: Thu, 26 Sep 2024 12:44:57 +0100
> From: Sean Whitton <spwhitton@spwhitton.name>
> 
> >  +++
> >  *** New command 'diff-revert-and-kill-hunk' bound to C-c M-r.
> > -This command reverse-applies the hunk at point, and then kills it.
> > -This is useful in buffers generated by C-x v = and C-x v D.
> > +This command reverts the hunk at point (i.e., applies the reverse of the
> > +hunk), and then removes the hunk from the diffs.  This is useful to undo
> > +commits when you are in buffers generated by 'C-x v =' and 'C-x v D'.
> 
>    ^^^^^^^
>    I think this should be "changes", right?

Why do you think so?  You mentioned VC commands, so I thought it was
logical to talk, about commits.

Re: master c86e7a29e78: ; Fix documentation of last change

[Sean Whitton]

Hello,

On Thu, Sep 26, 2024 at 04:39:42PM +0300, Eli Zaretskii wrote:
> > Date: Thu, 26 Sep 2024 12:44:57 +0100
> > From: Sean Whitton <spwhitton@spwhitton.name>
> > 
> > >  +++
> > >  *** New command 'diff-revert-and-kill-hunk' bound to C-c M-r.
> > > -This command reverse-applies the hunk at point, and then kills it.
> > > -This is useful in buffers generated by C-x v = and C-x v D.
> > > +This command reverts the hunk at point (i.e., applies the reverse of the
> > > +hunk), and then removes the hunk from the diffs.  This is useful to undo
> > > +commits when you are in buffers generated by 'C-x v =' and 'C-x v D'.
> > 
> >    ^^^^^^^
> >    I think this should be "changes", right?
> 
> Why do you think so?  You mentioned VC commands, so I thought it was
> logical to talk, about commits.

Right, but the two VCS commands I mention show uncommitted changes.

-- 
Sean Whitton

Re: master c86e7a29e78: ; Fix documentation of last change

[Eli Zaretskii]

> Date: Thu, 26 Sep 2024 15:19:58 +0100
> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: emacs-devel@gnu.org
> 
> Hello,
> 
> On Thu, Sep 26, 2024 at 04:39:42PM +0300, Eli Zaretskii wrote:
> > > Date: Thu, 26 Sep 2024 12:44:57 +0100
> > > From: Sean Whitton <spwhitton@spwhitton.name>
> > > 
> > > >  +++
> > > >  *** New command 'diff-revert-and-kill-hunk' bound to C-c M-r.
> > > > -This command reverse-applies the hunk at point, and then kills it.
> > > > -This is useful in buffers generated by C-x v = and C-x v D.
> > > > +This command reverts the hunk at point (i.e., applies the reverse of the
> > > > +hunk), and then removes the hunk from the diffs.  This is useful to undo
> > > > +commits when you are in buffers generated by 'C-x v =' and 'C-x v D'.
> > > 
> > >    ^^^^^^^
> > >    I think this should be "changes", right?
> > 
> > Why do you think so?  You mentioned VC commands, so I thought it was
> > logical to talk, about commits.
> 
> Right, but the two VCS commands I mention show uncommitted changes.

Maybe by default, not in general.  And there VC commands that show
committed changes, which can also be reverted by this new command,
right?

So maybe we should say "undo commits or changes".

Re: master c86e7a29e78: ; Fix documentation of last change

[Sean Whitton]

Hello,

On Thu 26 Sep 2024 at 05:25pm +03, Eli Zaretskii wrote:

>> Right, but the two VCS commands I mention show uncommitted changes.
>
> Maybe by default, not in general.  And there VC commands that show
> committed changes, which can also be reverted by this new command,
> right?

Good points.

> So maybe we should say "undo commits or changes".

Well, it can only undo commits when they consist of exactly one hunk, so
it would be misleading to say that it can undo commits in general.

I think just "changes" covers both uncommitted and committed changes
well, but we could have "undo uncommitted changes or revert committed
changes" ?

-- 
Sean Whitton

Re: master c86e7a29e78: ; Fix documentation of last change

[Sean Whitton]

Hello,

On Thu 26 Sep 2024 at 04:46pm +01, Sean Whitton wrote:

> I think just "changes" covers both uncommitted and committed changes
> well, but we could have "undo uncommitted changes or revert committed
> changes" ?

Actually, that's no good, because we use "revert" to mean undoing
*un*committed changes in VC.

So, I think "changes" is best, but let me know what you think.

-- 
Sean Whitton

Re: master c86e7a29e78: ; Fix documentation of last change

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: emacs-devel@gnu.org
> Date: Thu, 26 Sep 2024 16:48:30 +0100
> 
> Hello,
> 
> On Thu 26 Sep 2024 at 04:46pm +01, Sean Whitton wrote:
> 
> > I think just "changes" covers both uncommitted and committed changes
> > well, but we could have "undo uncommitted changes or revert committed
> > changes" ?
> 
> Actually, that's no good, because we use "revert" to mean undoing
> *un*committed changes in VC.

Not only that, please look at the manual node you updated.  It uses
"revert hunk", and that's IMO better than "reverse-apply".

Re: master c86e7a29e78: ; Fix documentation of last change

[Sean Whitton]

Hello,

On Thu, Sep 26, 2024 at 07:18:07PM +0300, Eli Zaretskii wrote:
> > From: Sean Whitton <spwhitton@spwhitton.name>
> > Cc: emacs-devel@gnu.org
> > Date: Thu, 26 Sep 2024 16:48:30 +0100
> > 
> > Hello,
> > 
> > On Thu 26 Sep 2024 at 04:46pm +01, Sean Whitton wrote:
> > 
> > > I think just "changes" covers both uncommitted and committed changes
> > > well, but we could have "undo uncommitted changes or revert committed
> > > changes" ?
> > 
> > Actually, that's no good, because we use "revert" to mean undoing
> > *un*committed changes in VC.
> 
> Not only that, please look at the manual node you updated.  It uses
> "revert hunk", and that's IMO better than "reverse-apply".

Indeed, given the name of the command, 'revert' is better.

I've installed a change to etc/NEWS that avoids the various problems we've
been discussing.

-- 
Sean Whitton

Re: master c86e7a29e78: ; Fix documentation of last change

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: emacs-devel@gnu.org
> Date: Thu, 26 Sep 2024 16:46:00 +0100
> 
> I think just "changes" covers both uncommitted and committed changes
> well, but we could have "undo uncommitted changes or revert committed
> changes" ?

SGTM, thanks.