GNU bug report logs -
#25105
26.0.50; diff navigation is broken
Previous Next
Reported by: Mark Oteiza <mvoteiza <at> udel.edu>
Date: Sun, 4 Dec 2016 15:14:02 UTC
Severity: normal
Tags: patch
Merged with 25400
Found in version 26.0.50
Done: Tino Calancha <tino.calancha <at> gmail.com>
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 25105 in the body.
You can then email your comments to 25105 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sun, 04 Dec 2016 15:14:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Mark Oteiza <mvoteiza <at> udel.edu>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sun, 04 Dec 2016 15:14:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
I am guessing this is a consequence of bug#17544. From -Q:
1. Do C-x v d RET = in a repository with a bunch of worktree changes
2. Hit n. Point is now at the top of the SECOND hunk
3. Go to end of buffer.
4. Hit p. Point is now at the top of the PENULTIMATE hunk
Further, from -Q:
1. Find a file generated by git format-patch. I had on hand the v2 patches
from bug#24966.
2. Hit M-n. Emacs complains "Can’t find the beginning of the file"
These are all regressions from Emacs 25.1
In GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
of 2016-12-04 built on logos
Repository revision: 35a86f0b6fe5634e94212964657c538739743d72
Configured using:
'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
--localstatedir=/var --without-gconf --with-modules
--with-x-toolkit=lucid 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe
-fstack-protector-strong -g -fvar-tracking-assignments -g
-fvar-tracking-assignments' CPPFLAGS=-D_FORTIFY_SOURCE=2
LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'
Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
LUCID X11 MODULES LIBSYSTEMD
Important settings:
value of $LC_COLLATE: C
value of $LANG: en_US.UTF-8
locale-coding-system: utf-8-unix
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sun, 04 Dec 2016 15:27:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Mark Oteiza <mvoteiza <at> udel.edu> writes:
> I am guessing this is a consequence of bug#17544. From -Q:
Yes, probably. There were some other problems pointed out at:
http://lists.gnu.org/archive/html/emacs-devel/2016-11/msg00738.html
Dima, any ideas?
Added indication that bug 25105 blocks24655
Request was from
Glenn Morris <rgm <at> gnu.org>
to
control <at> debbugs.gnu.org.
(Mon, 05 Dec 2016 03:16:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 05 Dec 2016 15:39:02 GMT)
Full text and
rfc822 format available.
Message #13 received at 25105 <at> debbugs.gnu.org (full text, mbox):
npostavs <at> users.sourceforge.net writes:
> Mark Oteiza <mvoteiza <at> udel.edu> writes:
>
>> I am guessing this is a consequence of bug#17544. From -Q:
>
> Yes, probably. There were some other problems pointed out at:
> http://lists.gnu.org/archive/html/emacs-devel/2016-11/msg00738.html
Hi. These are intentional. Justification:
When you're at the beginning of the buffer, you're logically already at
the first hunk. A justification of THIS is that if you try to apply the
hunk at point from beginning-of-buffer (C-c C-a) then the first hunk is
applied, and the point moves to the second hunk. This is the behavior
both before and after my changes.
I think the changes make diff-mode more consistent. If you disagree,
let's move the discussion to emacs-devel.
dima
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 05 Dec 2016 15:54:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 05.12.2016 17:38, Dima Kogan wrote:
> Hi. These are intentional. Justification:
Which are intentional? I've described some legitimate problems in the
emacs-devel email.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 05 Dec 2016 16:34:01 GMT)
Full text and
rfc822 format available.
Message #19 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On December 5, 2016 7:53:41 AM PST, Dmitry Gutov
<dgutov <at> yandex.ru> wrote:
>On 05.12.2016 17:38, Dima Kogan wrote:
>
>> Hi. These are intentional. Justification:
>
>Which are intentional? I've described some legitimate problems in the
>emacs-devel email.
Sorry I wasn't clear. The points in the bug report were intentional. The auto-refinement breakage you described on the list was not, and I'll look at fixing it later today or tomorrow. My mail farted, and the message to the list to that effect apparently didn't make it
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 05 Dec 2016 16:56:01 GMT)
Full text and
rfc822 format available.
Message #22 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 05/12/16 at 08:33am, Dima Kogan wrote:
> On December 5, 2016 7:53:41 AM PST, Dmitry Gutov
> <dgutov <at> yandex.ru> wrote:
> >On 05.12.2016 17:38, Dima Kogan wrote:
> >
> >> Hi. These are intentional. Justification:
> >
> >Which are intentional? I've described some legitimate problems in the
> >emacs-devel email.
>
> Sorry I wasn't clear. The points in the bug report were intentional.
> The auto-refinement breakage you described on the list was not, and
> I'll look at fixing it later today or tomorrow. My mail farted, and
> the message to the list to that effect apparently didn't make it
I find it very hard to believe that the behavior in the second recipe
is intentional.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 05 Dec 2016 17:50:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Mark Oteiza <mvoteiza <at> udel.edu> writes:
> I find it very hard to believe that the behavior in the second recipe
> is intentional.
I missed that part of the bug report. Sorry about that. That is a bug
that I haven't seen at all earlier. Will look at fixing it today or
tomorrow. Thanks for reporting.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Wed, 07 Dec 2016 07:30:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 25105 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Mark Oteiza <mvoteiza <at> udel.edu> writes:
> Fixing C-c C-a to DTRT is great, thanks, but I don't think the
> off-by-one navigation changes to "n" and "p" (diff-hunk-next,
> diff-hunk-prev) make sense. While it may have made fixing the issues
> mentioned in the commit message easier, the changes to what "n" and "p"
> do at the beginning and end of a diff are not documented, and I didn't
> see any discussion of it in the associated bug.
>
> I contend that the new behavior is inconsistent with the behavior of
> other outline/thing-with-headers type things in Emacs. outline-mode,
> org-mode, and rst-mode are the first ones that come to mind.
Can you give a specific example of interaction in any of these modes
that is analogous to the off-by-one behavior you're referring to? The
fundamental question is what hunk diff-mode should think the point is
looking at, when it is in some non-diff message above the first hunk.
The answer I chose for the new navigation logic is "first hunk". You
could also choose "invalid hunk, not a hunk at all", which would imply
that C-c C-a and M-ret and such shouldn't work there. Better suggestions
welcome.
> It's also not clear how the introduced oddity with auto-refine is going
> to be resolved, unless a way is found to autorefine the first hunk
> without there being any user interaction. Then opening a diff has
> inconsistent auto-refining from the start.
I don't use auto-refinement, so didn't notice the breakage. Will look at
it in a bit.
In the meantime, I'm attaching a patch to address the 2nd point in the
bug report (25105). This serves to treat the junk before the first hunk
(i.e. commit message from 'git format-patch') as a file header. Looks
reasonable?
[diff-mode-handle-initial-junk.patch (text/x-diff, inline)]
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 5b48c8d..41476bd 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -768,7 +768,7 @@ diff-beginning-of-file-and-junk
(setq prevfile nextfile))
(if (and previndex (numberp prevfile) (< previndex prevfile))
(setq prevfile previndex))
- (if (and (numberp prevfile) (<= prevfile start))
+ (if (numberp prevfile)
(progn
(goto-char prevfile)
;; Now skip backward over the leading junk we may have before the
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sun, 25 Dec 2016 06:58:01 GMT)
Full text and
rfc822 format available.
Message #31 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Dima Kogan <dima <at> secretsauce.net> writes:
> Mark Oteiza <mvoteiza <at> udel.edu> writes:
>
>> I find it very hard to believe that the behavior in the second recipe
>> is intentional.
>
> I missed that part of the bug report. Sorry about that. That is a bug
> that I haven't seen at all earlier. Will look at fixing it today or
> tomorrow. Thanks for reporting.
I pushed the fix to this. What do yall want to do about the other logic?
Are any of yall planning to revisit this? If not, let's close this bug.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sun, 25 Dec 2016 13:55:01 GMT)
Full text and
rfc822 format available.
Message #34 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 24/12/16 at 10:57pm, Dima Kogan wrote:
> Dima Kogan <dima <at> secretsauce.net> writes:
>
> > Mark Oteiza <mvoteiza <at> udel.edu> writes:
> >
> >> I find it very hard to believe that the behavior in the second recipe
> >> is intentional.
> >
> > I missed that part of the bug report. Sorry about that. That is a bug
> > that I haven't seen at all earlier. Will look at fixing it today or
> > tomorrow. Thanks for reporting.
>
> I pushed the fix to this. What do yall want to do about the other logic?
> Are any of yall planning to revisit this? If not, let's close this bug.
I will be revisiting this, just haven't gotten around to it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 01:15:02 GMT)
Full text and
rfc822 format available.
Message #37 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Mark Oteiza <mvoteiza <at> udel.edu> writes:
> I am guessing this is a consequence of bug#17544. From -Q:
>
> 1. Do C-x v d RET = in a repository with a bunch of worktree changes
> 2. Hit n. Point is now at the top of the SECOND hunk
> 3. Go to end of buffer.
> 4. Hit p. Point is now at the top of the PENULTIMATE hunk
FWIW, to me this behaviour is very annoying and it has being
around already a while without a fix (4 months).
I would propose to revert the commit causing this misbehaviour.
Then, once the patch is mature enough, it can be pushed again without
affecting users.
Dima Kogan <dima <at> secretsauce.net> writes:
> I pushed the fix to this. What do yall want to do about the other logic?
If for 'the other logic' you mean Mark's recipe above, then what i want
is:
above recipe behaves exactly as it does in Emacs-25; that behaviour is
very convenient to read diffs.
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 01:21:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Tino Calancha <tino.calancha <at> gmail.com> writes:
> Mark Oteiza <mvoteiza <at> udel.edu> writes:
>
>> I am guessing this is a consequence of bug#17544. From -Q:
>>
>> 1. Do C-x v d RET = in a repository with a bunch of worktree changes
>> 2. Hit n. Point is now at the top of the SECOND hunk
>> 3. Go to end of buffer.
>> 4. Hit p. Point is now at the top of the PENULTIMATE hunk
> FWIW, to me this behaviour is very annoying and it has being
> around already a while without a fix (4 months).
> I would propose to revert the commit causing this misbehaviour.
> Then, once the patch is mature enough, it can be pushed again without
> affecting users.
Hi.
This isn't a misbehavior, it's the whole point of the patch. We can
argue about whether it's an improvement or not, but if this is a "bug",
then the solution is a full revert.
The behavior I want is to always have a consistent idea of which hunk we
are currently on. In the recipe above, between steps 1 and 2 the point
is at bob. Both before and after the patch, the codes agree that we are
on the hunk 1. When you press 'n', I thus argue you should end up at
hunk 2. Similary, when you press C-c C-a; it should apply the first
hunk, and move to the second. And so on. What would you like?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 01:28:01 GMT)
Full text and
rfc822 format available.
Message #43 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Dima Kogan <dima <at> secretsauce.net> writes:
> The behavior I want is to always have a consistent idea of which hunk we
> are currently on.
Some more behaviors that I think are desirable are described in the
commit message of the main patch:
https://github.com/emacs-mirror/emacs/commit/2c8a7e50d24daf19e
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 02:59:01 GMT)
Full text and
rfc822 format available.
Message #46 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 06/12/16 at 11:29pm, Dima Kogan wrote:
> Mark Oteiza <mvoteiza <at> udel.edu> writes:
>
> > Fixing C-c C-a to DTRT is great, thanks, but I don't think the
> > off-by-one navigation changes to "n" and "p" (diff-hunk-next,
> > diff-hunk-prev) make sense. While it may have made fixing the issues
> > mentioned in the commit message easier, the changes to what "n" and "p"
> > do at the beginning and end of a diff are not documented, and I didn't
> > see any discussion of it in the associated bug.
> >
> > I contend that the new behavior is inconsistent with the behavior of
> > other outline/thing-with-headers type things in Emacs. outline-mode,
> > org-mode, and rst-mode are the first ones that come to mind.
>
> Can you give a specific example of interaction in any of these modes
> that is analogous to the off-by-one behavior you're referring to?
I wrote about how your changes are make diff-mode _not_ analogous.
> The
> fundamental question is what hunk diff-mode should think the point is
> looking at, when it is in some non-diff message above the first hunk.
> The answer I chose for the new navigation logic is "first hunk". You
> could also choose "invalid hunk, not a hunk at all", which would imply
> that C-c C-a and M-ret and such shouldn't work there. Better suggestions
> welcome.
One might argue that C-c C-a and friends in a file header should apply
all hunks in a file, or perhaps that there should actually be
diff-apply-file commands, etc.
The way n and p worked was not a bug, yet you gratuitously changed them,
and broke auto-refinement. Why do I have to now hit two keys to refine
the first hunk, and one key to refine the second?
> > It's also not clear how the introduced oddity with auto-refine is going
> > to be resolved, unless a way is found to autorefine the first hunk
> > without there being any user interaction. Then opening a diff has
> > inconsistent auto-refining from the start.
>
> I don't use auto-refinement, so didn't notice the breakage. Will look at
> it in a bit.
It's on by default, so this statement perplexes me.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 03:07:01 GMT)
Full text and
rfc822 format available.
Message #49 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 05/01/17 at 05:27pm, Dima Kogan wrote:
> Dima Kogan <dima <at> secretsauce.net> writes:
>
> > The behavior I want is to always have a consistent idea of which hunk we
> > are currently on.
>
> Some more behaviors that I think are desirable are described in the
> commit message of the main patch:
>
> https://github.com/emacs-mirror/emacs/commit/2c8a7e50d24daf19e
The only mention of the changes to navigation is "Better navigation
logic". Not documented in NEWS, and no tests for the corner cases.
I fail to see how fixing corner cases in diff-apply-hunk has anything to
do with diff-{file,hunk}-{next-prev}
At first glance, it looks like the following patch would restore the
previous behavior, however it completely breaks auto refinement.
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 9dfcd944bb..3442b01d12 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -625,7 +625,7 @@ diff--wrap-navigation
;; inner one does not, which breaks the loop.
(defun diff-hunk-prev (&optional count skip-hunk-start)
"Go to the previous COUNT'th hunk."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
+ (interactive (list (prefix-numeric-value current-prefix-arg) nil))
(diff--wrap-navigation
skip-hunk-start
"prev hunk"
@@ -636,7 +636,7 @@ diff-hunk-prev
(defun diff-hunk-next (&optional count skip-hunk-start)
"Go to the next COUNT'th hunk."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
+ (interactive (list (prefix-numeric-value current-prefix-arg) nil))
(diff--wrap-navigation
skip-hunk-start
"next hunk"
@@ -647,7 +647,7 @@ diff-hunk-next
(defun diff-file-prev (&optional count skip-hunk-start)
"Go to the previous COUNT'th file."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
+ (interactive (list (prefix-numeric-value current-prefix-arg) nil))
(diff--wrap-navigation
skip-hunk-start
"prev file"
@@ -658,7 +658,7 @@ diff-file-prev
(defun diff-file-next (&optional count skip-hunk-start)
"Go to the next COUNT'th file."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
+ (interactive (list (prefix-numeric-value current-prefix-arg) nil))
(diff--wrap-navigation
skip-hunk-start
"next file"
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 03:10:01 GMT)
Full text and
rfc822 format available.
Message #52 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 05/01/17 at 05:20pm, Dima Kogan wrote:
> Tino Calancha <tino.calancha <at> gmail.com> writes:
>
> > Mark Oteiza <mvoteiza <at> udel.edu> writes:
> >
> >> I am guessing this is a consequence of bug#17544. From -Q:
> >>
> >> 1. Do C-x v d RET = in a repository with a bunch of worktree changes
> >> 2. Hit n. Point is now at the top of the SECOND hunk
> >> 3. Go to end of buffer.
> >> 4. Hit p. Point is now at the top of the PENULTIMATE hunk
> > FWIW, to me this behaviour is very annoying and it has being
> > around already a while without a fix (4 months).
> > I would propose to revert the commit causing this misbehaviour.
> > Then, once the patch is mature enough, it can be pushed again without
> > affecting users.
>
> <snip> if this is a "bug",
> then the solution is a full revert.
With the number of actual bugs (email/format-patch/pre-diff content, and
auto refinement) the initial patch caused, perhaps this is best.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 03:51:02 GMT)
Full text and
rfc822 format available.
Message #55 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Thu, 5 Jan 2017, Mark Oteiza wrote:
> At first glance, it looks like the following patch would restore the
> previous behavior, however it completely breaks auto refinement.
>
> diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
> index 9dfcd944bb..3442b01d12 100644
> --- a/lisp/vc/diff-mode.el
> +++ b/lisp/vc/diff-mode.el
> @@ -625,7 +625,7 @@ diff--wrap-navigation
> ;; inner one does not, which breaks the loop.
> (defun diff-hunk-prev (&optional count skip-hunk-start)
> "Go to the previous COUNT'th hunk."
> - (interactive (list (prefix-numeric-value current-prefix-arg) t))
> + (interactive (list (prefix-numeric-value current-prefix-arg) nil))
> (diff--wrap-navigation
> skip-hunk-start
> "prev hunk"
> @@ -636,7 +636,7 @@ diff-hunk-prev
>
> (defun diff-hunk-next (&optional count skip-hunk-start)
> "Go to the next COUNT'th hunk."
> - (interactive (list (prefix-numeric-value current-prefix-arg) t))
> + (interactive (list (prefix-numeric-value current-prefix-arg) nil))
> (diff--wrap-navigation
> skip-hunk-start
> "next hunk"
> @@ -647,7 +647,7 @@ diff-hunk-next
>
> (defun diff-file-prev (&optional count skip-hunk-start)
> "Go to the previous COUNT'th file."
> - (interactive (list (prefix-numeric-value current-prefix-arg) t))
> + (interactive (list (prefix-numeric-value current-prefix-arg) nil))
> (diff--wrap-navigation
> skip-hunk-start
> "prev file"
> @@ -658,7 +658,7 @@ diff-file-prev
>
> (defun diff-file-next (&optional count skip-hunk-start)
> "Go to the next COUNT'th file."
> - (interactive (list (prefix-numeric-value current-prefix-arg) t))
> + (interactive (list (prefix-numeric-value current-prefix-arg) nil))
> (diff--wrap-navigation
> skip-hunk-start
> "next file"
Hi Mark,
i have checked out your patch and your right: it recovered the Emacs-25
behaviour in your snippet code.
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 04:17:02 GMT)
Full text and
rfc822 format available.
Message #58 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Mark Oteiza <mvoteiza <at> udel.edu> writes:
> On 05/01/17 at 05:27pm, Dima Kogan wrote:
>> Dima Kogan <dima <at> secretsauce.net> writes:
>>
>> > The behavior I want is to always have a consistent idea of which hunk we
>> > are currently on.
>>
>> Some more behaviors that I think are desirable are described in the
>> commit message of the main patch:
>>
>> https://github.com/emacs-mirror/emacs/commit/2c8a7e50d24daf19e
>
> The only mention of the changes to navigation is "Better navigation
> logic". Not documented in NEWS, and no tests for the corner cases.
I just re-read the commit message, and I think it's clear that it
touches navigation. I'm not a seasoned contributor to this project, so I
don't know what the policy is regarding NEWS. In either case, that is
irrelevant: we'd still be having this conversation.
Tests would be good, as always. However the diff-mode prior to this
patch had no tests, and I've been using this code every day for years.
And again, this doesn't matter. The issues in question aren't unintended
bugs, and they would have passed any tests I would have written.
> I fail to see how fixing corner cases in diff-apply-hunk has anything
> to do with diff-{file,hunk}-{next-prev}
The issues being fixed are making anything that operates on hunks more
consistent, so diff-{file,hunk}-{next-prev} are relevant.
> At first glance, it looks like the following patch would restore the
> previous behavior, however it completely breaks auto refinement.
>
> <snip>
If you want to restore the previous behavior, wouldn't a revert be
better? Or are you trying to restore only a subset of the previous
behavior?
> With the number of actual bugs (email/format-patch/pre-diff content,
> and auto refinement) the initial patch caused, perhaps this is best.
The email/format-patch issue has nothing to do with me; it has been a
problem for years. The way to "fix" auto-refinement is to invoke
auto-refinement in a diff-mode-hook, as suggested earlier. The bug
reporter didn't like that, and I don't know what they want. I'm not sure
where the pre-diff content issue came from. Likely it came up because
the patch that was in the BTS for years wasn't what ended up being
merged, so I haven't sufficiently tested it. Lesson learned.
I consider the current behavior a significant improvement in usability,
but if there's a consensus that it's a step backward, then I'll go back
to carrying this patch in my local tree. Let me ask the few people I
know who would be using this code at all to get at least anecdotal
feedback.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 04:23:02 GMT)
Full text and
rfc822 format available.
Message #61 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Hi Dima,
thanks for your prompt replay.
i am sorry, i didn't follow the discussion in Bug#17544, so i just
realized its effects once the patch was pushed; those effects
affect my everyday use of Emacs.
(Added as CC the people who joined Bug#17544).
On Thu, 5 Jan 2017, Dima Kogan wrote:
> This isn't a misbehavior, it's the whole point of the patch. We can
> argue about whether it's an improvement or not
My point is that Bug#17544 is not a bug, it's a feature. Your fix
just breaks the feature.
> if this is a "bug", then the solution is a full revert.
It might be the right thing to do. I was actually very happy with how
Emacs-25 deal with this issue.
Another posibility is the patch that Mark just sent to this thread.
> The behavior I want is to always have a consistent idea of which hunk we
> are currently on.
IMO, this is not a good idea. Things depend of the perspective. If you
are in one hunk or another it depends of what you want to do. That is
part of the feature.
>Navigation and use of diff buffers had several annoying corner cases that this
>patch fixes. These corner cases were largely due to inconsistent treatment of
>file headers. Say you have a diff such as this:
I disagree, ideed i found it very consistent (see below).
> --- aaa
> +++ bbb
> @@ -52,7 +52,7 @@
> hunk1
> @@ -74,7 +74,7 @@
> hunk2
> --- ccc
> +++ ddd
> @@ -608,6 +608,6 @@
> hunk3
> @@ -654,7 +654,7 @@
> hunk4
>
>The file headers here are the '---' and '+++' lines. With the point on such a
>line, hunk operations would sometimes refer to the next hunk and sometimes to
>the previous hunk. Most of the time it would be the previous hunk, which is not
>what the user would expect.
It seems some users expect it :-)
>This patch consistently treats such headers as the
>next hunk. So with this patch, if the point is on the '--- ccc' line, the point
>is seen as referring to hunk3.
it's totally consistent to not consider
the file header the same if you are not doing the same operation. If i am at
--- ccc
then, i want `diff-hunk-next' bring me to the line:
@@ -608,6 +608,6 @@
and `diff-hunk-prev' bring me to the line:
@@ -74,7 +74,7 @@
To this happen, in the first case the point must be considered at hunk2,
but in the second case, the point must be considered in hunk3.
To me, this is pretty consistent with the intended (and useful) behaviour.
Regards,
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 04:44:02 GMT)
Full text and
rfc822 format available.
Message #64 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Dima Kogan <dima <at> secretsauce.net> writes:
> Mark Oteiza <mvoteiza <at> udel.edu> writes:
>> I fail to see how fixing corner cases in diff-apply-hunk has anything
>> to do with diff-{file,hunk}-{next-prev}
>
> The issues being fixed are making anything that operates on hunks more
> consistent, so diff-{file,hunk}-{next-prev} are relevant.
and this entire thread is about the contention over changes specifically
done to diff-{file,hunk}-{next-prev}.
To quote myself: "Fixing C-c C-a to DTRT is great, thanks, but I don't
think the off-by-one navigation changes to "n" and "p" (diff-hunk-next,
diff-hunk-prev) make sense."
https://lists.gnu.org/archive/html/emacs-devel/2016-12/msg00222.html
>> At first glance, it looks like the following patch would restore the
>> previous behavior, however it completely breaks auto refinement.
>>
>> <snip>
>
> If you want to restore the previous behavior, wouldn't a revert be
> better? Or are you trying to restore only a subset of the previous
> behavior?
I did not submit it as a solution to the problem at hand. The fact that
the patch breaks auto-refinement means that I cannot define my own
commands to call (diff-{file-hunk}-{prev-next} ARG nil) and have it
work.
Put another way, your changes make it nigh impossible to get the
previous n,p,{,} back without breaking something.
>> With the number of actual bugs (email/format-patch/pre-diff content,
>> and auto refinement) the initial patch caused, perhaps this is best.
>
> The email/format-patch issue has nothing to do with me; it has been a
> problem for years.
Yes it did, as the second recipe in this bug and 6b6abe0d clearly show.
> The way to "fix" auto-refinement is to invoke
> auto-refinement in a diff-mode-hook, as suggested earlier. The bug
> reporter didn't like that <snip>
Probably because auto-refinement is default behavior that got broken.
> I'm not sure
> where the pre-diff content issue came from. Likely it came up because
> the patch that was in the BTS for years wasn't what ended up being
> merged, so I haven't sufficiently tested it. Lesson learned.
>
> I consider the current behavior a significant improvement in usability,
> but if there's a consensus that it's a step backward, then I'll go back
> to carrying this patch in my local tree. Let me ask the few people I
> know who would be using this code at all to get at least anecdotal
> feedback.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 07:56:01 GMT)
Full text and
rfc822 format available.
Message #67 received at 25105 <at> debbugs.gnu.org (full text, mbox):
> From: Dima Kogan <dima <at> secretsauce.net>
> Date: Thu, 05 Jan 2017 20:16:12 -0800
> Cc: 25105 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>,
> Tino Calancha <tino.calancha <at> gmail.com>, npostavs <at> users.sourceforge.net
>
> I consider the current behavior a significant improvement in usability,
> but if there's a consensus that it's a step backward, then I'll go back
> to carrying this patch in my local tree.
Another alternative is to have a customizable option which will let
users decide what behavior they want.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 08:04:01 GMT)
Full text and
rfc822 format available.
Message #70 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 6 Jan 2017, Eli Zaretskii wrote:
>> From: Dima Kogan <dima <at> secretsauce.net>
>> Date: Thu, 05 Jan 2017 20:16:12 -0800
>> Cc: 25105 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>,
>> Tino Calancha <tino.calancha <at> gmail.com>, npostavs <at> users.sourceforge.net
>>
>> I consider the current behavior a significant improvement in usability,
>> but if there's a consensus that it's a step backward, then I'll go back
>> to carrying this patch in my local tree.
>
> Another alternative is to have a customizable option which will let
> users decide what behavior they want.
That would be OK. I would suggest to set this option nil by default,
i.e., disable the new feature by default for backward compatibility.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 06 Jan 2017 14:15:01 GMT)
Full text and
rfc822 format available.
Message #73 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 06.01.2017 11:03, Tino Calancha wrote:
>> Another alternative is to have a customizable option which will let
>> users decide what behavior they want.
> That would be OK. I would suggest to set this option nil by default,
> i.e., disable the new feature by default for backward compatibility.
If separating behavior into two parts that are controlled by a switch
would be feasible (I'm not sure), it might be okay.
However, the new behavior also fixes what was undoubtedly a problem:
When point is a bob in a diff-mode buffer, `C-c C-a' applies the first
hunk, and then stops at its beginning (in Emacs 25 and earlier).
We would then give up on that fix, whereas I'd prefer to have a solution
eventually, if not now. But if we do, I estimate we might have the "old
fixed" behavior encroach on the "new different" behavior in certain
respects, making the code even more complex.
I've honestly thought that Dima's patch's main purpose was to fix that
bug. And everything else we now complain about are just implementation's
side-effects.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 07 Jan 2017 01:56:02 GMT)
Full text and
rfc822 format available.
Message #76 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 6 Jan 2017, Dmitry Gutov wrote:
> However, the new behavior also fixes what was undoubtedly a problem:
>
> When point is a bob in a diff-mode buffer, `C-c C-a' applies the first hunk,
> and then stops at its beginning (in Emacs 25 and earlier).
Honestly, i wasn't aware of `C-c C-a' functionality so i didn't
realized that aim of the patch.
> We would then give up on that fix, whereas I'd prefer to have a solution
> eventually, if not now. But if we do, I estimate we might have the "old
> fixed" behavior encroach on the "new different" behavior in certain respects,
> making the code even more complex.
I agree, `C-c C-a' or `M-k' is a good thing to have fixed. Eventually, i
would like to use such features.
> I've honestly thought that Dima's patch's main purpose was to fix that bug.
> And everything else we now complain about are just implementation's
> side-effects.
It seems you are right. IMO, we must aim to have the `C-c C-a' stuff
fixed, but preserving those behaviours that we are complaining here.
Until this aim is fulfilled, i would like to pospone this patch, or
to have a temporary solution as Mark's one in this thread.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 07 Jan 2017 02:06:01 GMT)
Full text and
rfc822 format available.
Message #79 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 07/01/17 at 10:54am, Tino Calancha wrote:
> On Fri, 6 Jan 2017, Dmitry Gutov wrote:
>
> > I've honestly thought that Dima's patch's main purpose was to fix that
> > bug. And everything else we now complain about are just implementation's
> > side-effects.
>
> It seems you are right. IMO, we must aim to have the `C-c C-a' stuff fixed,
> but preserving those behaviours that we are complaining here.
> Until this aim is fulfilled, i would like to pospone this patch, or to have
> a temporary solution as Mark's one in this thread.
I don't consider my patch a solution, as it basically disables diff
refinement due to the design of Dima's patch.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 07 Jan 2017 09:53:02 GMT)
Full text and
rfc822 format available.
Message #82 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06.01.2017 11:03, Tino Calancha wrote:
>
> I've honestly thought that Dima's patch's main purpose was to fix that
> bug. And everything else we now complain about are just implementation's
> side-effects.
The goals are described in the git commit message.
The way these goals were met was to make everything consistent, with my
interpretation of what that means.
Taking the new behavior for M-k, C-c C-a and keeping the old behavior
for M-n/M-p is weird. Example:
Let's look at the commands required to apply hunks 1 and 2 in a buffer
versus just hunk 2. Assuming we're at bob, and assuming we're using the
new code.
Applying hunks 1 and 2: C-c C-a C-c C-a; i.e. apply 1, apply 2
Applying hunk 2 only: M-n C-c C-a; i.e. skip 1, apply 2
If M-n works the way it did before, then you need to invoke M-n twice to
apply hunk 2 only. I claim this is weird, since it should require only
one command to "skip 1". This is what is meant by a "consistent"
behavior.
To reinterate, my desired outcome is to have the current code in place,
and to hook auto-refinement in a diff-mode-hook to make that work in a
way that makes sense. I'm fine with a switch that lets you pick what you
want. As stated above, I don't think a hybrid approach makes sense,
since it takes one weird thing and converts it to another thing that's
weird in a different way.
Furthermore, I'd like to get some feedback from more than just the few
people active on this thread. If there's a lopsided preference to the
old approach, we can simply revert and move on.
dima
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 07 Jan 2017 11:17:01 GMT)
Full text and
rfc822 format available.
Message #85 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Sat, 7 Jan 2017, Dima Kogan wrote:
Dear Dima,
i appreciate very much your efforts to improve diff-mode.
In particular, i like 1. and 2. in your commit message.
That are useful goals. I just disagree with the implementation.
In summary, i believe a better approach is to get 1., 2.
without affecting the behaviour of `n', `p'.
> 1. It should be possible to place the point in the middle of a diff
> buffer, and press M-k repeatedly to kill hunks in the order they appear
> in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then
> hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is
> fine. However, with the point on hunk2, it'd kill hunk2 then hunk1.
> This is fixed by this patch.
>
> 2. Similarly, it should be possible to apply hunks in order. Previously
> with the point at the start, C-c C-a would apply the hunk1, then move
> the point to the first @@ header, and thus C-c C-a would try to apply
> the same hunk again.
Your patch is invasive: it should be possible to get 1) an 2) above without
redefining a long standing behaviour for `diff-hunk-next' and `diff-hunk-prev'.
In addition, that change in `n' and `p' would deserve a more prominent explanation
in the commit message, f.i., a new dot 3., so people would be aware of
this subtle change.
>Let's look at the commands required to apply hunks 1 and 2 in a buffer
>versus just hunk 2. Assuming we're at bob, and assuming we're using the
>new code.
>
> Applying hunks 1 and 2: C-c C-a C-c C-a; i.e. apply 1, apply 2
> Applying hunk 2 only: M-n C-c C-a; i.e. skip 1, apply 2
>
>If M-n works the way it did before, then you need to invoke M-n twice to
>apply hunk 2 only. I claim this is weird, since it should require only
>one command to "skip 1". This is what is meant by a "consistent"
>behavior.
Well, not everyone follow that logic. I like to read carefully the hunks
of a patch _before_ decide to apply then. I can do that with easy using `n' and `p'
keys. If the commit message is long, as in your commit 2c8a7e5, then an user need
`n' to jump to the first hunk and read it; but after your patch the first hunk
is skipped. Then the user need to be a believer and apply it without seeing
it using `C-c C-a'; non believer users, might prefer to rewind and read the hunk
as follows:
M-! git show 2c8a7e5 RET
C-o C-x C-q
M-x diff-mode RET
n
;; we are at 2nd hunk, i.e., at line:
@@ -570,7 +570,102 @@ diff--auto-refine-data
;; Assuming no split window, so we can see clearly the first hunk;
;; after looking at it carefully, we decided is good, so let's apply it:
p ; jump to 1st hunk: this is _extra_ compared with Emacs-25!
C-c C-a
So, it might be argued that this patch force users to gratuitously push
_always_ an extra `p' to read/apply the first hunk of _every_ patch.
That makes hunk navigation much less pleasant.
>Furthermore, I'd like to get some feedback from more than just the few
>people active on this thread. If there's a lopsided preference to the
>old approach, we can simply revert and move on.
As a rule of thumb, if i get N people here uncomfortable with one of my
patches on master branch, i believe that can easily be translated
to (* 100 N) people once the patch is in a stable release (at least).
>I'm fine with a switch that lets you pick what you
>want. As stated above, I don't think a hybrid approach makes sense,
>since it takes one weird thing and converts it to another thing that's
>weird in a different way.
FWIW, I am working in an alternative patch: it would satisfy your 1. and 2.
while respecting `n' and `p'. Stay tune!
Best regards,
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 07 Jan 2017 22:17:01 GMT)
Full text and
rfc822 format available.
Message #88 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Tino Calancha <tino.calancha <at> gmail.com> writes:
> On Sat, 7 Jan 2017, Dima Kogan wrote:
>
>>Let's look at the commands required to apply hunks 1 and 2 in a buffer
>>versus just hunk 2. Assuming we're at bob, and assuming we're using the
>>new code.
>>
>> Applying hunks 1 and 2: C-c C-a C-c C-a; i.e. apply 1, apply 2
>> Applying hunk 2 only: M-n C-c C-a; i.e. skip 1, apply 2
>>
>>If M-n works the way it did before, then you need to invoke M-n twice to
>>apply hunk 2 only. I claim this is weird, since it should require only
>>one command to "skip 1". This is what is meant by a "consistent"
>>behavior.
> Well, not everyone follow that logic. I like to read carefully the hunks
> of a patch _before_ decide to apply then. I can do that with easy using `n' and `p'
> keys. If the commit message is long, as in your commit 2c8a7e5, then an user need
> `n' to jump to the first hunk and read it; but after your patch the first hunk
> is skipped. Then the user need to be a believer and apply it without seeing
> it using `C-c C-a'; non believer users, might prefer to rewind and read the hunk
> as follows:
> M-! git show 2c8a7e5 RET
> C-o C-x C-q
> M-x diff-mode RET
> n
> ;; we are at 2nd hunk, i.e., at line:
> @@ -570,7 +570,102 @@ diff--auto-refine-data
> ;; Assuming no split window, so we can see clearly the first hunk;
> ;; after looking at it carefully, we decided is good, so let's apply it:
> p ; jump to 1st hunk: this is _extra_ compared with Emacs-25!
> C-c C-a
>
> So, it might be argued that this patch force users to gratuitously push
> _always_ an extra `p' to read/apply the first hunk of _every_ patch.
> That makes hunk navigation much less pleasant.
This clearly shows that it would be useful to have some binding that
jumps to the beginning of the current hunk. But that's not the same as
moving to the 'next' hunk, which is what 'n' ostensibly is supposed to
do.
>>Furthermore, I'd like to get some feedback from more than just the few
>>people active on this thread. If there's a lopsided preference to the
>>old approach, we can simply revert and move on.
> As a rule of thumb, if i get N people here uncomfortable with one of my
> patches on master branch, i believe that can easily be translated
> to (* 100 N) people once the patch is in a stable release (at least).
Right. The choice is whether to revert the new behavior entirely, or to
leave it in with a switch. Since we're looking at a small sample here,
it's not clear which is the right move.
>>I'm fine with a switch that lets you pick what you
>>want. As stated above, I don't think a hybrid approach makes sense,
>>since it takes one weird thing and converts it to another thing that's
>>weird in a different way.
> FWIW, I am working in an alternative patch: it would satisfy your 1. and 2.
> while respecting `n' and `p'. Stay tune!
Great!
dima
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 07 Jan 2017 22:28:02 GMT)
Full text and
rfc822 format available.
Message #91 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 08.01.2017 01:16, Dima Kogan wrote:
> This clearly shows that it would be useful to have some binding that
> jumps to the beginning of the current hunk. But that's not the same as
> moving to the 'next' hunk, which is what 'n' ostensibly is supposed to
> do.
`n' is the easiest key to press, so it should perform navigation that
the users will find most useful, in general.
If the behavior does not correspond to the command name, it would be
better to rename the command, or change its docstring, or etc.
But I don't think `diff-hunk-next' jumping to the beginning of the first
hunk when invoked at the file header, is in any way odd. After all,
point wasn't inside a hunk before.
> Right. The choice is whether to revert the new behavior entirely, or to
> leave it in with a switch. Since we're looking at a small sample here,
> it's not clear which is the right move.
Note, however, that nobody else has stepped forward to explicitly
support the new behavior.
If you get around to polling Emacs users at your workplace, or a meetup,
or so on, please let us know.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Wed, 11 Jan 2017 04:39:02 GMT)
Full text and
rfc822 format available.
Message #94 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:
> In a buffer with more than one hunk, if I'm in the middle of hunk number
> N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
> number N-1 rather than to the header of hunk N.
>
> This is contrary to the usual behavior of Emacs's navigation commands.
>As pointed out elsewhere, it's particularly obnoxious from EOB (in which
>case, you're not really "within N" but you're virtually on "the header
>of the non-existent hunk N+1", so going to the header of N-1 is really
>wrong).
>
>I also dislike the fact that M-n doesn't let me get to EOB.
Following patch reverts commit 2c8a7e5. Then it fixes dots 1. and 2.
described in the commit message of 2c8a7e5, i.e., Bug#17544.
This patch preserves the original definitions for 'diff-hunk-prev'
and 'diff-hunk-next'.
After applying locally this patch, you might want to do:
git diff 2c8a7e5^ HEAD lisp/vc/diff-mode.el
to see more clearly how it solves Bug#17544.
Regards,
Tino
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From a2dfaf390f4069b4e948dd0df84450359e7a0d92 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Wed, 11 Jan 2017 13:20:04 +0900
Subject: [PATCH] Fix Bugs 25105 and 25400
Revert 2c8a7e5 and implement a new fix for Bug#17544.
This patch satisfies 1. and 2. in 2c8a7e5 commit message.
The original definitions for 'diff-hunk-prev' and 'diff-hunk-next'
are preserved.
* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate.
(diff-beginning-of-hunk): Use it.
(diff-apply-hunk): Jump to beginning of hunk before apply the hunk.
(diff-hunk-kill, diff-file-kill): Jump to beginning of hunk after kill.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
---
lisp/vc/diff-mode.el | 241 ++++++++++++++++++---------------------------------
1 file changed, 83 insertions(+), 158 deletions(-)
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 9dfcd944bb..3fc4713f0f 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -498,22 +498,55 @@ diff-end-of-hunk
;; The return value is used by easy-mmode-define-navigation.
(goto-char (or end (point-max)))))
+;; "index ", "old mode", "new mode", "new file mode" and
+;; "deleted file mode" are output by git-diff.
+(defconst diff-file-junk-re
+ "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+
+;; If point is in a diff header, then return beginning
+;; of hunk position otherwise return nil.
+(defun diff--at-diff-header-p ()
+ "Return non-nil if point is inside a diff header."
+ (let ((regexp-hunk diff-hunk-header-re)
+ (regexp-file diff-file-header-re)
+ (regexp-junk diff-file-junk-re)
+ (orig (point)))
+ (catch 'headerp
+ (save-excursion
+ (forward-line 0)
+ (when (looking-at regexp-hunk) ; Hunk header.
+ (throw 'headerp (point)))
+ (forward-line -1)
+ (when (re-search-forward regexp-file (point-at-eol 4) t) ; File header.
+ (forward-line 0)
+ (throw 'headerp (point)))
+ (goto-char orig)
+ (forward-line 0)
+ (when (looking-at regexp-junk) ; Git diff junk.
+ (while (and (looking-at regexp-junk)
+ (not (bobp)))
+ (forward-line -1))
+ (re-search-forward regexp-file nil t)
+ (forward-line 0)
+ (throw 'headerp (point)))) nil)))
+
(defun diff-beginning-of-hunk (&optional try-harder)
"Move back to the previous hunk beginning, and return its position.
If point is in a file header rather than a hunk, advance to the
next hunk if TRY-HARDER is non-nil; otherwise signal an error."
(beginning-of-line)
- (if (looking-at diff-hunk-header-re)
+ (if (looking-at diff-hunk-header-re) ; At hunk header.
(point)
- (forward-line 1)
- (condition-case ()
- (re-search-backward diff-hunk-header-re)
- (error
- (unless try-harder
- (error "Can't find the beginning of the hunk"))
- (diff-beginning-of-file-and-junk)
- (diff-hunk-next)
- (point)))))
+ (let ((pos (diff--at-diff-header-p))
+ (regexp diff-hunk-header-re))
+ (cond (pos ; At junk diff header.
+ (if try-harder
+ (goto-char pos)
+ (error "Can't find the beginning of the hunk")))
+ ((re-search-backward regexp nil t)) ; In the middle of a hunk.
+ ((re-search-forward regexp nil t) ; At first hunk header.
+ (forward-line 0))
+ (t (error "Can't find the beginning of the hunk"))))))
(defun diff-unified-hunk-p ()
(save-excursion
@@ -551,124 +584,26 @@ diff--auto-refine-data
;; Define diff-{hunk,file}-{prev,next}
(easy-mmode-define-navigation
- diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view)
+ diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view
+ (when diff-auto-refine-mode
+ (unless (prog1 diff--auto-refine-data
+ (setq diff--auto-refine-data
+ (cons (current-buffer) (point-marker))))
+ (run-at-time 0.0 nil
+ (lambda ()
+ (when diff--auto-refine-data
+ (let ((buffer (car diff--auto-refine-data))
+ (point (cdr diff--auto-refine-data)))
+ (setq diff--auto-refine-data nil)
+ (with-local-quit
+ (when (buffer-live-p buffer)
+ (with-current-buffer buffer
+ (save-excursion
+ (goto-char point)
+ (diff-refine-hunk))))))))))))
(easy-mmode-define-navigation
- diff--internal-file diff-file-header-re "file" diff-end-of-file)
-
-(defun diff--wrap-navigation (skip-hunk-start
- what orig
- header-re goto-start-func count)
- "Wrap diff-{hunk,file}-{next,prev} for more intuitive behavior.
-Override the default diff-{hunk,file}-{next,prev} implementation
-by skipping any lines that are associated with this hunk/file but
-precede the hunk-start marker. For instance, a diff file could
-contain
-
-diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
-index 923de9a..6b1c24f 100644
---- a/lisp/vc/diff-mode.el
-+++ b/lisp/vc/diff-mode.el
-@@ -590,6 +590,22 @@
-.......
-
-If a point is on 'index', then the point is considered to be in
-this first hunk. Move the point to the @@... marker before
-executing the default diff-hunk-next/prev implementation to move
-to the NEXT marker."
- (if (not skip-hunk-start)
- (funcall orig count)
-
- (let ((start (point)))
- (funcall goto-start-func)
-
- ;; Trap the error.
- (condition-case nil
- (funcall orig count)
- (error nil))
-
- (when (not (looking-at header-re))
- (goto-char start)
- (user-error (format "No %s" what)))
-
- ;; We successfully moved to the next/prev hunk/file. Apply the
- ;; auto-refinement if needed
- (when diff-auto-refine-mode
- (unless (prog1 diff--auto-refine-data
- (setq diff--auto-refine-data
- (cons (current-buffer) (point-marker))))
- (run-at-time 0.0 nil
- (lambda ()
- (when diff--auto-refine-data
- (let ((buffer (car diff--auto-refine-data))
- (point (cdr diff--auto-refine-data)))
- (setq diff--auto-refine-data nil)
- (with-local-quit
- (when (buffer-live-p buffer)
- (with-current-buffer buffer
- (save-excursion
- (goto-char point)
- (diff-refine-hunk))))))))))))))
-
-;; These functions all take a skip-hunk-start argument which controls
-;; whether we skip pre-hunk-start text or not. In interactive uses we
-;; always want to do this, but the simple behavior is still necessary
-;; to, for example, avoid an infinite loop:
-;;
-;; diff-hunk-next calls
-;; diff--wrap-navigation calls
-;; diff-bounds-of-hunk calls
-;; diff-beginning-of-hunk calls
-;; diff-hunk-next
-;;
-;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the
-;; inner one does not, which breaks the loop.
-(defun diff-hunk-prev (&optional count skip-hunk-start)
- "Go to the previous COUNT'th hunk."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "prev hunk"
- 'diff--internal-hunk-prev
- diff-hunk-header-re
- (lambda () (goto-char (car (diff-bounds-of-hunk))))
- count))
-
-(defun diff-hunk-next (&optional count skip-hunk-start)
- "Go to the next COUNT'th hunk."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "next hunk"
- 'diff--internal-hunk-next
- diff-hunk-header-re
- (lambda () (goto-char (car (diff-bounds-of-hunk))))
- count))
-
-(defun diff-file-prev (&optional count skip-hunk-start)
- "Go to the previous COUNT'th file."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "prev file"
- 'diff--internal-file-prev
- diff-file-header-re
- (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
- count))
-
-(defun diff-file-next (&optional count skip-hunk-start)
- "Go to the next COUNT'th file."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "next file"
- 'diff--internal-file-next
- diff-file-header-re
- (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
- count))
-
-
-
+ diff-file diff-file-header-re "file" diff-end-of-file)
(defun diff-bounds-of-hunk ()
"Return the bounds of the diff hunk at point.
@@ -679,13 +614,12 @@ diff-bounds-of-hunk
(let ((pos (point))
(beg (diff-beginning-of-hunk t))
(end (diff-end-of-hunk)))
- (cond ((> end pos)
+ (cond ((>= end pos)
(list beg end))
;; If this hunk ends above POS, consider the next hunk.
((re-search-forward diff-hunk-header-re nil t)
(list (match-beginning 0) (diff-end-of-hunk)))
- ;; There's no next hunk, so just take the one we have.
- (t (list beg end))))))
+ (t (error "No hunk found"))))))
(defun diff-bounds-of-file ()
"Return the bounds of the file segment at point.
@@ -731,12 +665,8 @@ diff-hunk-kill
hunk-bounds))
(inhibit-read-only t))
(apply 'kill-region bounds)
- (goto-char (car bounds))))
-
-;; "index ", "old mode", "new mode", "new file mode" and
-;; "deleted file mode" are output by git-diff.
-(defconst diff-file-junk-re
- "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+ (goto-char (car bounds))
+ (diff-beginning-of-hunk t)))
(defun diff-beginning-of-file-and-junk ()
"Go to the beginning of file-related diff-info.
@@ -771,7 +701,7 @@ diff-beginning-of-file-and-junk
(setq prevfile nextfile))
(if (and previndex (numberp prevfile) (< previndex prevfile))
(setq prevfile previndex))
- (if (numberp prevfile)
+ (if (and (numberp prevfile) (<= prevfile start))
(progn
(goto-char prevfile)
;; Now skip backward over the leading junk we may have before the
@@ -789,7 +719,8 @@ diff-file-kill
"Kill current file's hunks."
(interactive)
(let ((inhibit-read-only t))
- (apply 'kill-region (diff-bounds-of-file))))
+ (apply 'kill-region (diff-bounds-of-file)))
+ (diff-beginning-of-hunk t))
(defun diff-kill-junk ()
"Kill spurious empty diffs."
@@ -1373,7 +1304,7 @@ diff-post-command-hook
;; it's safer not to do it on big changes, e.g. when yanking a big
;; diff, or when the user edits the header, since we might then
;; screw up perfectly correct values. --Stef
- (diff-beginning-of-hunk)
+ (diff-beginning-of-hunk t)
(let* ((style (if (looking-at "\\*\\*\\*") 'context))
(start (line-beginning-position (if (eq style 'context) 3 2)))
(mid (if (eq style 'context)
@@ -1764,9 +1695,8 @@ diff-find-source-location
SWITCHED is non-nil if the patch is already applied.
NOPROMPT, if non-nil, means not to prompt the user."
(save-excursion
- (let* ((hunk-bounds (diff-bounds-of-hunk))
- (other (diff-xor other-file diff-jump-to-old-file))
- (char-offset (- (point) (goto-char (car hunk-bounds))))
+ (let* ((other (diff-xor other-file diff-jump-to-old-file))
+ (char-offset (- (point) (diff-beginning-of-hunk t)))
;; Check that the hunk is well-formed. Otherwise diff-mode and
;; the user may disagree on what constitutes the hunk
;; (e.g. because an empty line truncates the hunk mid-course),
@@ -1775,7 +1705,7 @@ diff-find-source-location
;; Suppress check when NOPROMPT is non-nil (Bug#3033).
(_ (unless noprompt (diff-sanity-check-hunk)))
(hunk (buffer-substring
- (point) (cadr hunk-bounds)))
+ (point) (save-excursion (diff-end-of-hunk) (point))))
(old (diff-hunk-text hunk reverse char-offset))
(new (diff-hunk-text hunk (not reverse) char-offset))
;; Find the location specification.
@@ -1838,6 +1768,7 @@ diff-apply-hunk
With a prefix argument, REVERSE the hunk."
(interactive "P")
+ (diff-beginning-of-hunk t)
(pcase-let ((`(,buf ,line-offset ,pos ,old ,new ,switched)
;; Sometimes we'd like to have the following behavior: if
;; REVERSE go to the new file, otherwise go to the old.
@@ -1883,15 +1814,8 @@ diff-apply-hunk
;; Display BUF in a window
(set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
(diff-hunk-status-msg line-offset (diff-xor switched reverse) nil)
-
- ;; Advance to the next hunk with skip-hunk-start set to t
- ;; because we want the behavior of moving to the next logical
- ;; hunk, not the original behavior where were would sometimes
- ;; stay on the current hunk. This is the behavior we get when
- ;; navigating through hunks interactively, and we want it when
- ;; applying hunks too (see http://debbugs.gnu.org/17544).
(when diff-advance-after-apply-hunk
- (diff-hunk-next nil t))))))
+ (diff-hunk-next))))))
(defun diff-test-hunk (&optional reverse)
@@ -1972,15 +1896,14 @@ diff-current-defun
(defun diff-ignore-whitespace-hunk ()
"Re-diff the current hunk, ignoring whitespace differences."
(interactive)
- (let* ((hunk-bounds (diff-bounds-of-hunk))
- (char-offset (- (point) (goto-char (car hunk-bounds))))
+ (let* ((char-offset (- (point) (diff-beginning-of-hunk t)))
(opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b")))
(line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)")
(error "Can't find line number"))
(string-to-number (match-string 1))))
(inhibit-read-only t)
(hunk (delete-and-extract-region
- (point) (cadr hunk-bounds)))
+ (point) (save-excursion (diff-end-of-hunk) (point))))
(lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1.
(file1 (make-temp-file "diff1"))
(file2 (make-temp-file "diff2"))
@@ -2067,14 +1990,16 @@ diff-refine-hunk
(interactive)
(require 'smerge-mode)
(save-excursion
- (let* ((hunk-bounds (diff-bounds-of-hunk))
- (style (progn (goto-char (car hunk-bounds))
- (diff-hunk-style))) ;Skips the hunk header as well.
+ (diff-beginning-of-hunk t)
+ (let* ((start (point))
+ (style (diff-hunk-style)) ;Skips the hunk header as well.
(beg (point))
- (end (cadr hunk-bounds))
(props-c '((diff-mode . fine) (face diff-refine-changed)))
(props-r '((diff-mode . fine) (face diff-refine-removed)))
- (props-a '((diff-mode . fine) (face diff-refine-added))))
+ (props-a '((diff-mode . fine) (face diff-refine-added)))
+ ;; Be careful to go back to `start' so diff-end-of-hunk gets
+ ;; to read the hunk header's line info.
+ (end (progn (goto-char start) (diff-end-of-hunk) (point))))
(remove-overlays beg end 'diff-mode 'fine)
--
2.11.0
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.5)
of 2017-01-10
Repository revision: fa0a2b4e7c81f57aecc1d94df00588a4dd5c281d
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Wed, 11 Jan 2017 23:29:02 GMT)
Full text and
rfc822 format available.
Message #97 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 11/01/17 at 01:38pm, Tino Calancha wrote:
> Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:
>
> > In a buffer with more than one hunk, if I'm in the middle of hunk number
> > N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
> > number N-1 rather than to the header of hunk N.
> >
> > This is contrary to the usual behavior of Emacs's navigation commands.
> >As pointed out elsewhere, it's particularly obnoxious from EOB (in which
> >case, you're not really "within N" but you're virtually on "the header
> >of the non-existent hunk N+1", so going to the header of N-1 is really
> >wrong).
> >
> >I also dislike the fact that M-n doesn't let me get to EOB.
>
> Following patch reverts commit 2c8a7e5. Then it fixes dots 1. and 2.
> described in the commit message of 2c8a7e5, i.e., Bug#17544.
> This patch preserves the original definitions for 'diff-hunk-prev'
> and 'diff-hunk-next'.
>
> After applying locally this patch, you might want to do:
> git diff 2c8a7e5^ HEAD lisp/vc/diff-mode.el
> to see more clearly how it solves Bug#17544.
Works for me AFAICT. Thanks for working on it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Wed, 11 Jan 2017 23:35:02 GMT)
Full text and
rfc822 format available.
Message #100 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Hi Tino,
Thanks for doing this. However, I'd prefer two separate commits: one
reverting 2c8a7e5 (if that is what we are doing), and one with your
actual changes.
That would make the history easier to read, and would probably improve
'git blame' results as well.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Thu, 12 Jan 2017 03:55:01 GMT)
Full text and
rfc822 format available.
Message #103 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Thu, 12 Jan 2017, Dmitry Gutov wrote:
> I'd prefer two separate commits:
> one reverting 2c8a7e5 (if that is what we are doing),
> and one with your actual changes.
>
> That would make the history easier to read, and would probably improve 'git
> blame' results as well.
Definitely. Thanks for the suggestion!
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 4c0b6cb87438a5c812a4718452c2537d827a74a4 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Thu, 12 Jan 2017 12:46:03 +0900
Subject: [PATCH 1/2] ; Revert "Improve diff-mode navigation/manipulation"
This reverts commit 2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085.
This change causes regressions:
https://lists.gnu.org/archive/html/emacs-devel/2016-11/msg00738.html
Fixes: debbugs:25105, 25400.
---
lisp/vc/diff-mode.el | 174 ++++++++++-----------------------------------------
1 file changed, 34 insertions(+), 140 deletions(-)
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 9dfcd944bb..44556ddd4a 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -551,124 +551,26 @@ diff--auto-refine-data
;; Define diff-{hunk,file}-{prev,next}
(easy-mmode-define-navigation
- diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view)
+ diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view
+ (when diff-auto-refine-mode
+ (unless (prog1 diff--auto-refine-data
+ (setq diff--auto-refine-data
+ (cons (current-buffer) (point-marker))))
+ (run-at-time 0.0 nil
+ (lambda ()
+ (when diff--auto-refine-data
+ (let ((buffer (car diff--auto-refine-data))
+ (point (cdr diff--auto-refine-data)))
+ (setq diff--auto-refine-data nil)
+ (with-local-quit
+ (when (buffer-live-p buffer)
+ (with-current-buffer buffer
+ (save-excursion
+ (goto-char point)
+ (diff-refine-hunk))))))))))))
(easy-mmode-define-navigation
- diff--internal-file diff-file-header-re "file" diff-end-of-file)
-
-(defun diff--wrap-navigation (skip-hunk-start
- what orig
- header-re goto-start-func count)
- "Wrap diff-{hunk,file}-{next,prev} for more intuitive behavior.
-Override the default diff-{hunk,file}-{next,prev} implementation
-by skipping any lines that are associated with this hunk/file but
-precede the hunk-start marker. For instance, a diff file could
-contain
-
-diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
-index 923de9a..6b1c24f 100644
---- a/lisp/vc/diff-mode.el
-+++ b/lisp/vc/diff-mode.el
-@@ -590,6 +590,22 @@
-.......
-
-If a point is on 'index', then the point is considered to be in
-this first hunk. Move the point to the @@... marker before
-executing the default diff-hunk-next/prev implementation to move
-to the NEXT marker."
- (if (not skip-hunk-start)
- (funcall orig count)
-
- (let ((start (point)))
- (funcall goto-start-func)
-
- ;; Trap the error.
- (condition-case nil
- (funcall orig count)
- (error nil))
-
- (when (not (looking-at header-re))
- (goto-char start)
- (user-error (format "No %s" what)))
-
- ;; We successfully moved to the next/prev hunk/file. Apply the
- ;; auto-refinement if needed
- (when diff-auto-refine-mode
- (unless (prog1 diff--auto-refine-data
- (setq diff--auto-refine-data
- (cons (current-buffer) (point-marker))))
- (run-at-time 0.0 nil
- (lambda ()
- (when diff--auto-refine-data
- (let ((buffer (car diff--auto-refine-data))
- (point (cdr diff--auto-refine-data)))
- (setq diff--auto-refine-data nil)
- (with-local-quit
- (when (buffer-live-p buffer)
- (with-current-buffer buffer
- (save-excursion
- (goto-char point)
- (diff-refine-hunk))))))))))))))
-
-;; These functions all take a skip-hunk-start argument which controls
-;; whether we skip pre-hunk-start text or not. In interactive uses we
-;; always want to do this, but the simple behavior is still necessary
-;; to, for example, avoid an infinite loop:
-;;
-;; diff-hunk-next calls
-;; diff--wrap-navigation calls
-;; diff-bounds-of-hunk calls
-;; diff-beginning-of-hunk calls
-;; diff-hunk-next
-;;
-;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the
-;; inner one does not, which breaks the loop.
-(defun diff-hunk-prev (&optional count skip-hunk-start)
- "Go to the previous COUNT'th hunk."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "prev hunk"
- 'diff--internal-hunk-prev
- diff-hunk-header-re
- (lambda () (goto-char (car (diff-bounds-of-hunk))))
- count))
-
-(defun diff-hunk-next (&optional count skip-hunk-start)
- "Go to the next COUNT'th hunk."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "next hunk"
- 'diff--internal-hunk-next
- diff-hunk-header-re
- (lambda () (goto-char (car (diff-bounds-of-hunk))))
- count))
-
-(defun diff-file-prev (&optional count skip-hunk-start)
- "Go to the previous COUNT'th file."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "prev file"
- 'diff--internal-file-prev
- diff-file-header-re
- (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
- count))
-
-(defun diff-file-next (&optional count skip-hunk-start)
- "Go to the next COUNT'th file."
- (interactive (list (prefix-numeric-value current-prefix-arg) t))
- (diff--wrap-navigation
- skip-hunk-start
- "next file"
- 'diff--internal-file-next
- diff-file-header-re
- (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
- count))
-
-
-
+ diff-file diff-file-header-re "file" diff-end-of-file)
(defun diff-bounds-of-hunk ()
"Return the bounds of the diff hunk at point.
@@ -679,13 +581,12 @@ diff-bounds-of-hunk
(let ((pos (point))
(beg (diff-beginning-of-hunk t))
(end (diff-end-of-hunk)))
- (cond ((> end pos)
+ (cond ((>= end pos)
(list beg end))
;; If this hunk ends above POS, consider the next hunk.
((re-search-forward diff-hunk-header-re nil t)
(list (match-beginning 0) (diff-end-of-hunk)))
- ;; There's no next hunk, so just take the one we have.
- (t (list beg end))))))
+ (t (error "No hunk found"))))))
(defun diff-bounds-of-file ()
"Return the bounds of the file segment at point.
@@ -771,7 +672,7 @@ diff-beginning-of-file-and-junk
(setq prevfile nextfile))
(if (and previndex (numberp prevfile) (< previndex prevfile))
(setq prevfile previndex))
- (if (numberp prevfile)
+ (if (and (numberp prevfile) (<= prevfile start))
(progn
(goto-char prevfile)
;; Now skip backward over the leading junk we may have before the
@@ -1764,9 +1665,8 @@ diff-find-source-location
SWITCHED is non-nil if the patch is already applied.
NOPROMPT, if non-nil, means not to prompt the user."
(save-excursion
- (let* ((hunk-bounds (diff-bounds-of-hunk))
- (other (diff-xor other-file diff-jump-to-old-file))
- (char-offset (- (point) (goto-char (car hunk-bounds))))
+ (let* ((other (diff-xor other-file diff-jump-to-old-file))
+ (char-offset (- (point) (diff-beginning-of-hunk t)))
;; Check that the hunk is well-formed. Otherwise diff-mode and
;; the user may disagree on what constitutes the hunk
;; (e.g. because an empty line truncates the hunk mid-course),
@@ -1775,7 +1675,7 @@ diff-find-source-location
;; Suppress check when NOPROMPT is non-nil (Bug#3033).
(_ (unless noprompt (diff-sanity-check-hunk)))
(hunk (buffer-substring
- (point) (cadr hunk-bounds)))
+ (point) (save-excursion (diff-end-of-hunk) (point))))
(old (diff-hunk-text hunk reverse char-offset))
(new (diff-hunk-text hunk (not reverse) char-offset))
;; Find the location specification.
@@ -1883,15 +1783,8 @@ diff-apply-hunk
;; Display BUF in a window
(set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
(diff-hunk-status-msg line-offset (diff-xor switched reverse) nil)
-
- ;; Advance to the next hunk with skip-hunk-start set to t
- ;; because we want the behavior of moving to the next logical
- ;; hunk, not the original behavior where were would sometimes
- ;; stay on the current hunk. This is the behavior we get when
- ;; navigating through hunks interactively, and we want it when
- ;; applying hunks too (see http://debbugs.gnu.org/17544).
(when diff-advance-after-apply-hunk
- (diff-hunk-next nil t))))))
+ (diff-hunk-next))))))
(defun diff-test-hunk (&optional reverse)
@@ -1972,15 +1865,14 @@ diff-current-defun
(defun diff-ignore-whitespace-hunk ()
"Re-diff the current hunk, ignoring whitespace differences."
(interactive)
- (let* ((hunk-bounds (diff-bounds-of-hunk))
- (char-offset (- (point) (goto-char (car hunk-bounds))))
+ (let* ((char-offset (- (point) (diff-beginning-of-hunk t)))
(opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b")))
(line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)")
(error "Can't find line number"))
(string-to-number (match-string 1))))
(inhibit-read-only t)
(hunk (delete-and-extract-region
- (point) (cadr hunk-bounds)))
+ (point) (save-excursion (diff-end-of-hunk) (point))))
(lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1.
(file1 (make-temp-file "diff1"))
(file2 (make-temp-file "diff2"))
@@ -2067,14 +1959,16 @@ diff-refine-hunk
(interactive)
(require 'smerge-mode)
(save-excursion
- (let* ((hunk-bounds (diff-bounds-of-hunk))
- (style (progn (goto-char (car hunk-bounds))
- (diff-hunk-style))) ;Skips the hunk header as well.
+ (diff-beginning-of-hunk t)
+ (let* ((start (point))
+ (style (diff-hunk-style)) ;Skips the hunk header as well.
(beg (point))
- (end (cadr hunk-bounds))
(props-c '((diff-mode . fine) (face diff-refine-changed)))
(props-r '((diff-mode . fine) (face diff-refine-removed)))
- (props-a '((diff-mode . fine) (face diff-refine-added))))
+ (props-a '((diff-mode . fine) (face diff-refine-added)))
+ ;; Be careful to go back to `start' so diff-end-of-hunk gets
+ ;; to read the hunk header's line info.
+ (end (progn (goto-char start) (diff-end-of-hunk) (point))))
(remove-overlays beg end 'diff-mode 'fine)
--
2.11.0
From a5809529fac90741c6ede3fc66dfdac1e011434c Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Thu, 12 Jan 2017 12:46:25 +0900
Subject: [PATCH 2/2] Fix Bug#17544
* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate.
(diff-beginning-of-hunk): Use it.
(diff-apply-hunk): Jump to beginning of hunk before apply the hunk.
(diff-hunk-kill, diff-file-kill): Jump to beginning of hunk after kill.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
---
lisp/vc/diff-mode.el | 67 ++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 49 insertions(+), 18 deletions(-)
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 44556ddd4a..3fc4713f0f 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -498,22 +498,55 @@ diff-end-of-hunk
;; The return value is used by easy-mmode-define-navigation.
(goto-char (or end (point-max)))))
+;; "index ", "old mode", "new mode", "new file mode" and
+;; "deleted file mode" are output by git-diff.
+(defconst diff-file-junk-re
+ "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+
+;; If point is in a diff header, then return beginning
+;; of hunk position otherwise return nil.
+(defun diff--at-diff-header-p ()
+ "Return non-nil if point is inside a diff header."
+ (let ((regexp-hunk diff-hunk-header-re)
+ (regexp-file diff-file-header-re)
+ (regexp-junk diff-file-junk-re)
+ (orig (point)))
+ (catch 'headerp
+ (save-excursion
+ (forward-line 0)
+ (when (looking-at regexp-hunk) ; Hunk header.
+ (throw 'headerp (point)))
+ (forward-line -1)
+ (when (re-search-forward regexp-file (point-at-eol 4) t) ; File header.
+ (forward-line 0)
+ (throw 'headerp (point)))
+ (goto-char orig)
+ (forward-line 0)
+ (when (looking-at regexp-junk) ; Git diff junk.
+ (while (and (looking-at regexp-junk)
+ (not (bobp)))
+ (forward-line -1))
+ (re-search-forward regexp-file nil t)
+ (forward-line 0)
+ (throw 'headerp (point)))) nil)))
+
(defun diff-beginning-of-hunk (&optional try-harder)
"Move back to the previous hunk beginning, and return its position.
If point is in a file header rather than a hunk, advance to the
next hunk if TRY-HARDER is non-nil; otherwise signal an error."
(beginning-of-line)
- (if (looking-at diff-hunk-header-re)
+ (if (looking-at diff-hunk-header-re) ; At hunk header.
(point)
- (forward-line 1)
- (condition-case ()
- (re-search-backward diff-hunk-header-re)
- (error
- (unless try-harder
- (error "Can't find the beginning of the hunk"))
- (diff-beginning-of-file-and-junk)
- (diff-hunk-next)
- (point)))))
+ (let ((pos (diff--at-diff-header-p))
+ (regexp diff-hunk-header-re))
+ (cond (pos ; At junk diff header.
+ (if try-harder
+ (goto-char pos)
+ (error "Can't find the beginning of the hunk")))
+ ((re-search-backward regexp nil t)) ; In the middle of a hunk.
+ ((re-search-forward regexp nil t) ; At first hunk header.
+ (forward-line 0))
+ (t (error "Can't find the beginning of the hunk"))))))
(defun diff-unified-hunk-p ()
(save-excursion
@@ -632,12 +665,8 @@ diff-hunk-kill
hunk-bounds))
(inhibit-read-only t))
(apply 'kill-region bounds)
- (goto-char (car bounds))))
-
-;; "index ", "old mode", "new mode", "new file mode" and
-;; "deleted file mode" are output by git-diff.
-(defconst diff-file-junk-re
- "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+ (goto-char (car bounds))
+ (diff-beginning-of-hunk t)))
(defun diff-beginning-of-file-and-junk ()
"Go to the beginning of file-related diff-info.
@@ -690,7 +719,8 @@ diff-file-kill
"Kill current file's hunks."
(interactive)
(let ((inhibit-read-only t))
- (apply 'kill-region (diff-bounds-of-file))))
+ (apply 'kill-region (diff-bounds-of-file)))
+ (diff-beginning-of-hunk t))
(defun diff-kill-junk ()
"Kill spurious empty diffs."
@@ -1274,7 +1304,7 @@ diff-post-command-hook
;; it's safer not to do it on big changes, e.g. when yanking a big
;; diff, or when the user edits the header, since we might then
;; screw up perfectly correct values. --Stef
- (diff-beginning-of-hunk)
+ (diff-beginning-of-hunk t)
(let* ((style (if (looking-at "\\*\\*\\*") 'context))
(start (line-beginning-position (if (eq style 'context) 3 2)))
(mid (if (eq style 'context)
@@ -1738,6 +1768,7 @@ diff-apply-hunk
With a prefix argument, REVERSE the hunk."
(interactive "P")
+ (diff-beginning-of-hunk t)
(pcase-let ((`(,buf ,line-offset ,pos ,old ,new ,switched)
;; Sometimes we'd like to have the following behavior: if
;; REVERSE go to the new file, otherwise go to the old.
--
2.11.0
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.5)
of 2017-01-12
Repository revision: d40073f017ffb3dee2266f356c127ef587c40b71
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 03:36:01 GMT)
Full text and
rfc822 format available.
Message #106 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 12.01.2017 06:53, Tino Calancha wrote:
> Definitely. Thanks for the suggestion!
Could you re-send the patches at attachments? Or just push them to a
branch. I wish the first one could be produced with a simple 'git
revert', but that leads to a conflict.
I get "Can't find the text to patch" with the second one, though.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 03:56:01 GMT)
Full text and
rfc822 format available.
Message #109 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 13 Jan 2017, Dmitry Gutov wrote:
> Could you re-send the patches at attachments? Or just push them to a branch.
I have pushed those changes into following new branch:
scratch/calancha-revert-2c8a7e5
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 04:26:02 GMT)
Full text and
rfc822 format available.
Message #112 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 13 Jan 2017, Dmitry Gutov wrote:
> I wish the first one could be produced with a simple 'git revert', but that
> leads to a conflict.
Sorry for the confusin. I made the revert manually.
In addition to commit 2c8a7e5, i've reverted following
other related commits:
e5ef59b87da5c2ddfa22f7342efe29b3eea6ed97
6b6abe0dba6a9a2e5f78aac3814421886e7a184f
73349822cbd6e50526eda9c75453584d73dfca83
a283d655db88cdcc8cb53d8e2578e1cdf751c84b
61c6a10e3110490dadac4577cc540053341ff25c
2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085
In scratch/calancha-revert-2c8a7e5 branch,
diff-mode.el history is as follows:
##### _before_ my changes #####
;; initial state for diff-mode.el is as in commit:
1f5592572887fe15e5b660bc60e66a7ab7c624cd ; w/ copyright year 2016
;; Update copyright year and we are at
;; 3e30cda89474209716c6e16a1a81d02877c95a2b
;; i.e., first commit in my new branch).
;; then, second commit (0beb7d2968ab76878eb3be26f2d749977fdcaa2f)
;; add my fix for Bug#17544.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 05:57:01 GMT)
Full text and
rfc822 format available.
Message #115 received at 25105 <at> debbugs.gnu.org (full text, mbox):
merge 25400 25105
tags 25400 patch
quit
I merged the bugs, and dropped #25400 from the cc list, because I'm
getting duplicate emails everytime. While I'm on a meta topic, sorry
for being hasty with merging the original patch that changed diff
movement like that. I didn't fully grasp it, nor tested like I should
have done.
Tino Calancha <tino.calancha <at> gmail.com> writes:
>
> ;; then, second commit (0beb7d2968ab76878eb3be26f2d749977fdcaa2f)
> ;; add my fix for Bug#17544.
I think the commit summary could be a bit friendlier, otherwise it looks
good to me.
Merged 25105 25400.
Request was from
npostavs <at> users.sourceforge.net
to
control <at> debbugs.gnu.org.
(Fri, 13 Jan 2017 05:57:02 GMT)
Full text and
rfc822 format available.
Added tag(s) patch.
Request was from
npostavs <at> users.sourceforge.net
to
control <at> debbugs.gnu.org.
(Fri, 13 Jan 2017 05:57:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 06:27:02 GMT)
Full text and
rfc822 format available.
Message #122 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 13 Jan 2017, npostavs <at> users.sourceforge.net wrote:
> merge 25400 25105
> tags 25400 patch
> quit
>
> I merged the bugs, and dropped #25400 from the cc list, because I'm
> getting duplicate emails everytime.
Thank you. Sorry for the duplicated e-mails :-(
> While I'm on a meta topic, sorry
> for being hasty with merging the original patch that changed diff
> movement like that. I didn't fully grasp it, nor tested like I should
> have done.
No problem. Indeed you are helping a lot with the bugs. Thank you very
much.
>> ;; then, second commit (0beb7d2968ab76878eb3be26f2d749977fdcaa2f)
>> ;; add my fix for Bug#17544.
>
> I think the commit summary could be a bit friendlier, otherwise it looks
> good to me.
Thanks. I agree: if i read my own commit after some months, probably i
will not fully understand it.
I propose following more verbose one:
Fix Bug#17544
* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate; return non-nil when point
is inside a hunk header, a file header, or within a line
matching diff-file-junk-re.
(diff-beginning-of-hunk): Use it.
Check if the point is inside a diff header, in the middle of a hunk,
or before the first hunk.
(diff-apply-hunk): Call diff-beginning-of-hunk with non-nil arg
before apply the hunk.
(diff-hunk-kill, diff-file-kill):
Call diff-beginning-of-hunk with non-nil arg after kill the hunks.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 06:42:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Tino Calancha <tino.calancha <at> gmail.com> writes:
>>
>> I merged the bugs, and dropped #25400 from the cc list, because I'm
>> getting duplicate emails everytime.
> Thank you. Sorry for the duplicated e-mails :-(
Hmm, I'm still getting duplicates. Maybe I shouldn't have merged? Oh well.
>>
>> I think the commit summary could be a bit friendlier, otherwise it looks
>> good to me.
> Thanks. I agree: if i read my own commit after some months, probably
> i will not fully understand it.
> I propose following more verbose one:
>
> Fix Bug#17544
I was thinking more about this summary line. And maybe add some higher
level explanation before the ChangeLog entry about the problems being
solved?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Fri, 13 Jan 2017 06:55:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 13 Jan 2017, npostavs <at> users.sourceforge.net wrote:
> Tino Calancha <tino.calancha <at> gmail.com> writes:
>
>>>
>>> I merged the bugs, and dropped #25400 from the cc list, because I'm
>>> getting duplicate emails everytime.
>> Thank you. Sorry for the duplicated e-mails :-(
>
> Hmm, I'm still getting duplicates. Maybe I shouldn't have merged? Oh well.
Could be because i answered the e-mail to you and CC to Bug#25105? If you
are registered to that bug you might get 2 e-mails.
>>> I think the commit summary could be a bit friendlier, otherwise it looks
>>> good to me.
>> Thanks. I agree: if i read my own commit after some months, probably
>> i will not fully understand it.
>> I propose following more verbose one:
>>
>> Fix Bug#17544
>
> I was thinking more about this summary line. And maybe add some higher
> level explanation before the ChangeLog entry about the problems being
> solved?
How about copy verbatim the comentary from Dima in 2c8a7e5? He explained
very clearly the problem, and i am solving the same thing.
Mention to the regressions should must appear in the previous commit,
where i do the actual revert.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 14 Jan 2017 01:49:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 25105 <at> debbugs.gnu.org (full text, mbox):
Tino Calancha <tino.calancha <at> gmail.com> writes:
> On Fri, 13 Jan 2017, npostavs <at> users.sourceforge.net wrote:
>
>> Hmm, I'm still getting duplicates. Maybe I shouldn't have merged? Oh well.
> Could be because i answered the e-mail to you and CC to Bug#25105? If you
> are registered to that bug you might get 2 e-mails.
I think that's the normal case, and usually I don't get duplicated
mails.
>> I was thinking more about this summary line. And maybe add some higher
>> level explanation before the ChangeLog entry about the problems being
>> solved?
> How about copy verbatim the comentary from Dima in 2c8a7e5? He explained
> very clearly the problem, and i am solving the same thing.
> Mention to the regressions should must appear in the previous commit,
> where i do the actual revert.
Yes, that looks good, though if I understand correctly, the important
difference is that the navigation commands are not changed in your
patch, only the manipulation ones, right?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 14 Jan 2017 03:12:01 GMT)
Full text and
rfc822 format available.
Message #134 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 13.01.2017 06:55, Tino Calancha wrote:
> I have pushed those changes into following new branch:
> scratch/calancha-revert-2c8a7e5
I've tried it out (but not the changes discussed in this thread later),
and the behavior is much better. Thanks!
Please apply it to master sooner rather than later.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 14 Jan 2017 03:17:02 GMT)
Full text and
rfc822 format available.
Message #137 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 13.01.2017 07:25, Tino Calancha wrote:
> Sorry for the confusin. I made the revert manually.
> In addition to commit 2c8a7e5, i've reverted following
> other related commits:
>
> e5ef59b87da5c2ddfa22f7342efe29b3eea6ed97
> 6b6abe0dba6a9a2e5f78aac3814421886e7a184f
> 73349822cbd6e50526eda9c75453584d73dfca83
> a283d655db88cdcc8cb53d8e2578e1cdf751c84b
> 61c6a10e3110490dadac4577cc540053341ff25c
> 2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085
On that subject, maybe reverting each of them separately would be better.
Although I've tried how that affects 'git blame' in practice, and it
makes no difference.
Maybe our Changelog-generating script could take reverts into account
(and skip commits that have been explicitly reverted later), if it
doesn't do that already.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Sat, 14 Jan 2017 05:48:01 GMT)
Full text and
rfc822 format available.
Message #140 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Fri, 13 Jan 2017, npostavs <at> users.sourceforge.net wrote:
> Tino Calancha <tino.calancha <at> gmail.com> writes:
>> How about copy verbatim the comentary from Dima in 2c8a7e5? He explained
>> very clearly the problem, and i am solving the same thing.
>> Mention to the regressions should must appear in the previous commit,
>> where i do the actual revert.
>
> Yes, that looks good, though if I understand correctly, the important
> difference is that the navigation commands are not changed in your
> patch, only the manipulation ones, right?
That's right.
Essentially, my patch fix 1. and 2. in the commit message of 2c8a7e5.
I am going to prepare one draft for the commit message based on Dima's
message by Monday or Tuesday: i will drop mentions to the navigation
commands. Then i will show it here so that people in this thread can
give me comments/corrections to make the message clear for everyone.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 16 Jan 2017 06:27:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Sat, 14 Jan 2017, Tino Calancha wrote:
> Essentially, my patch fix 1. and 2. in the commit message of 2c8a7e5.
> I am going to prepare one draft for the commit message based on Dima's
> message by Monday or Tuesday: i will drop mentions to the navigation
> commands. Then i will show it here so that people in this thread can
> give me comments/corrections to make the message clear for everyone.
OK, below is my draft for the second commit.
Please don't hesitate to send me comments to make it more clear or/and
fix some broken grammar.
;; For the first commit, the revert of 2c8a7e5, i propose to keep the same
;; as before. Let me know in case you want to change it as well. Maybe
;; you want this commit list all the commits that it reverts?
;; Currently it just mentions 2c8a7e5, but it doesn't say a word about
;; the 'children commits':
;; e5ef59b8, 6b6abe0d, 73349822, a283d655 and 61c6a10e.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Improve diff-mode manipulation commands
Fix Bug#17544.
The use of `diff-apply-hunk' and `diff-hunk-kill' had several annoying
corner cases that this patch fixes. These corner cases were largely due
to inconsistent treatment of diff headers.
Consider following diff:
diff --git a/foo b/bar
index 44556ddd4a..3fc4713f0f 100644
--- a/foo
+++ b/bar
@@ -52,7 +52,7 @@
hunk1
@@ -74,7 +74,7 @@
hunk2
diff --git a/baz b/qux
index a8c3fcca2f..6b316c4073 100644
--- a/baz
+++ b/qux
@@ -608,6 +608,6 @@
hunk3
@@ -654,7 +654,7 @@
hunk4
The lines starting and ending with '@@' are the hunk headers.
The lines starting with '---' and '+++' are the file headers.
In addition, Git diffs add extra lines as those starting with
'diff' and 'index' above. Let's call these lines Git header.
From now on, these three headers are referred together as the diff header,
that is,
diff header = 'Git + file + hunk' headers.
Commands `diff-apply-hunk' and `diff-hunk-kill' behaves differently
depending on the point position within a diff header.
1. If point is at hunk1 header, '@@ -52,7 +52,7 @@' above, then
`diff-apply-hunk' moves point to hunk2 header,
i.e., '@@ -74,7 +74,7 @@' line.
If point is at '--- a/foo' line, then `diff-apply-hunk' moves point
to hunk1 header.
2. If point is at hunk3 header or its file header, then
`diff-hunk-kill' deletes hunk3.
If point is at the beginning of its Git header,
i.e., 'diff --git a/baz b/qux' line, then `diff-hunk-kill' deletes hunk2.
After this patch the behaviour of these commands is independent of the
point position in the diff header. Then, it's possible to apply hunks
in order. It's also possible to press M-k repeatedly to kill hunks in
the order they appear in the buffer.
* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate.
(diff-beginning-of-hunk): Use it.
(diff-apply-hunk): Jump to beginning of hunk before apply the hunk.
(diff-hunk-kill, diff-file-kill): Jump to beginning of hunk after kill.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Tue, 17 Jan 2017 23:25:02 GMT)
Full text and
rfc822 format available.
Message #146 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 16.01.2017 09:26, Tino Calancha wrote:
> OK, below is my draft for the second commit.
> Please don't hesitate to send me comments to make it more clear or/and
> fix some broken grammar.
Here's a fresh idea. If we're not going to worry about (not) showing
Dima's commit message in the generated change log, maybe the new message
should talk about tweaking the code, and not retell the original message.
Or the users will read it twice, basically. But only those users that
read change logs, of course.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Wed, 18 Jan 2017 06:12:01 GMT)
Full text and
rfc822 format available.
Message #149 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On Wed, 18 Jan 2017, Dmitry Gutov wrote:
> On 16.01.2017 09:26, Tino Calancha wrote:
>
>> OK, below is my draft for the second commit.
>> Please don't hesitate to send me comments to make it more clear or/and
>> fix some broken grammar.
>
> Here's a fresh idea. If we're not going to worry about (not) showing Dima's
> commit message in the generated change log, maybe the new message should talk
> about tweaking the code, and not retell the original message.
>
> Or the users will read it twice, basically. But only those users that read
> change logs, of course.
OK, it's reasonable.
See my new draft for commit 0beb7d2968ab76878eb3be26f2d749977fdcaa2f
in branch scratch/calancha-revert-2c8a7e5:
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Make diff-apply-hunk independent of point within a diff header
Make diff-apply-hunk and diff-hunk-kill independent of the point
position in a diff header (Bug#17544).
This change allows to apply hunks in order. It also makes possible to
press M-k repeatedly to kill hunks in the order they appear in the buffer.
* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate; return non-nil when point
is inside a hunk header, a file header, or within a line
matching diff-file-junk-re.
(diff-beginning-of-hunk): Use it.
Check if the point is inside a diff header, in the middle of a hunk,
or before the first hunk.
(diff-apply-hunk): Call diff-beginning-of-hunk with non-nil arg
before apply the hunk.
(diff-hunk-kill, diff-file-kill):
Call diff-beginning-of-hunk with non-nil arg after kill the hunks.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Reply sent
to
Tino Calancha <tino.calancha <at> gmail.com>:
You have taken responsibility.
(Sat, 21 Jan 2017 03:03:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Mark Oteiza <mvoteiza <at> udel.edu>:
bug acknowledged by developer.
(Sat, 21 Jan 2017 03:03:01 GMT)
Full text and
rfc822 format available.
Message #154 received at 25105-done <at> debbugs.gnu.org (full text, mbox):
On Sat, 14 Jan 2017, Dmitry Gutov wrote:
> On 13.01.2017 06:55, Tino Calancha wrote:
>
>> I have pushed those changes into following new branch:
>> scratch/calancha-revert-2c8a7e5
>
> I've tried it out (but not the changes discussed in this thread later), and
> the behavior is much better. Thanks!
>
> Please apply it to master sooner rather than later.
Pushed the fix to master branch as commits:
1508b538fd8f8c2e00aadcea42ac36013fad02e3
e5e42cefd7f2eb47d2c8660a7a317e8b08d36a82
Reply sent
to
Tino Calancha <tino.calancha <at> gmail.com>:
You have taken responsibility.
(Sat, 21 Jan 2017 03:03:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Stefan Monnier <monnier <at> IRO.UMontreal.CA>:
bug acknowledged by developer.
(Sat, 21 Jan 2017 03:03:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#25105; Package
emacs.
(Mon, 23 Jan 2017 03:43:02 GMT)
Full text and
rfc822 format available.
Message #162 received at 25105 <at> debbugs.gnu.org (full text, mbox):
On 21.01.2017 06:02, Tino Calancha wrote:
> Pushed the fix to master branch as commits:
> 1508b538fd8f8c2e00aadcea42ac36013fad02e3
> e5e42cefd7f2eb47d2c8660a7a317e8b08d36a82
Thank you very much. I can use the master branch again. :)
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Mon, 20 Feb 2017 12:24:03 GMT)
Full text and
rfc822 format available.
This bug report was last modified 8 years and 123 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.