New commands for browsing diffs easily

New commands for browsing diffs easily

[Lars Magne Ingebrigtsen]

I use `C-x v =' a lot, and one thing I've missed for ages is the
ability to just walk through the diff history.  That is, after seeing
that the diff in question wasn't really the one I was looking for, I
just want to see the diffs that comes before or after.

So this diff adds that.  I'm not quite sure that this is the best
implementation -- calling vc functions from diff mode might be a bit
naughty.  Or perhaps not.

I bound the commands to `M-C-n' and `M-C-p' as all the more likely key
strokes were already taken.

Index: ChangeLog
===================================================================
RCS file: /sources/emacs/emacs/lisp/ChangeLog,v
retrieving revision 1.9381
diff --unified -r1.9381 ChangeLog
--- ChangeLog	11 Apr 2006 00:08:56 -0000	1.9381
+++ ChangeLog	11 Apr 2006 08:13:24 -0000
@@ -1,3 +1,11 @@
+2006-04-11  Lars Magne Ingebrigtsen  <larsi@gnus.org>
+
+	* vc.el (vc-buffer-revisions): New variable.
+	(vc-version-diff): Record the revisions.
+
+	* diff-mode.el (diff-display-previous-diff)
+	(diff-display-next-diff, diff-display-new-diff): New commands. 
+
 2006-04-10  Bill Wohler  <wohler@newt.com>
 
 	* custom.el (defcustom, custom-handle-keyword): Add
Index: diff-mode.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/diff-mode.el,v
retrieving revision 1.85
diff --unified -r1.85 diff-mode.el
--- diff-mode.el	21 Mar 2006 10:15:39 -0000	1.85
+++ diff-mode.el	11 Apr 2006 08:13:24 -0000
@@ -135,6 +135,8 @@
     ("R" . diff-reverse-direction)
     ("U" . diff-context->unified)
     ("C" . diff-unified->context)
+    ("\C-p" . diff-display-previous-diff)
+    ("\C-n" . diff-display-next-diff)
     ("q" . quit-window))
   "Basic keymap for `diff-mode', bound to various prefix keys.")
 
@@ -1376,6 +1378,30 @@
       (delete-file file1)
       (delete-file file2))))
 
+(defun diff-display-previous-diff ()
+  "Display the the diff between the two previous revisions."
+  (interactive)
+  (diff-display-new-diff vc-parent-buffer
+			 (vc-call previous-version
+				  (buffer-file-name vc-parent-buffer)
+				  (car vc-buffer-revisions))
+			 (car vc-buffer-revisions)))
+
+(defun diff-display-next-diff ()
+  "Display the the diff between the two next revisions."
+  (interactive)
+  (when (zerop (length (cadr vc-buffer-revisions)))
+    (error "At the end of the revision list"))
+  (diff-display-new-diff vc-parent-buffer
+			 (cadr vc-buffer-revisions)
+			 (vc-call next-version
+				  (buffer-file-name vc-parent-buffer)
+				  (cadr vc-buffer-revisions))))
+
+(defun diff-display-new-diff (parent-buffer rev1 rev2)
+  (set-buffer parent-buffer)
+  (vc-version-diff (buffer-file-name (current-buffer)) rev1 rev2))
+
 ;; provide the package
 (provide 'diff-mode)
 
Index: vc.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc.el,v
retrieving revision 1.414
diff --unified -r1.414 vc.el
--- vc.el	7 Feb 2006 16:59:01 -0000	1.414
+++ vc.el	11 Apr 2006 08:13:25 -0000
@@ -716,6 +716,8 @@
 (put 'vc-parent-buffer 'permanent-local t)
 (defvar vc-parent-buffer-name nil)
 (put 'vc-parent-buffer-name 'permanent-local t)
+(defvar vc-buffer-revisions nil)
+(put 'vc-buffer-revisions 'permanent-local t)
 
 (defvar vc-disable-async-diff nil
   "VC sets this to t locally to disable some async diff operations.
@@ -1755,6 +1757,8 @@
     ;; buffer should affect the diff command.
     (vc-diff-internal file rev1 rev2))
   (set-buffer "*vc-diff*")
+  ;; Record the revisions used to create this buffer
+  (set (make-local-variable 'vc-buffer-revisions) (list rev1 rev2))
   (if (and (zerop (buffer-size))
 	   (not (get-buffer-process (current-buffer))))
       (progn


-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: New commands for browsing diffs easily

[Nick Roberts]

 > I use `C-x v =' a lot, and one thing I've missed for ages is the
 > ability to just walk through the diff history.  That is, after seeing
 > that the diff in question wasn't really the one I was looking for, I
 > just want to see the diffs that comes before or after.

Does log-view-mode do what you want?  Do `C-x v l' and then `d' on the
appropriate line of the vc-change-log buffer (See VC status node in the manual).
 
 > So this diff adds that.  I'm not quite sure that this is the best
 > implementation -- calling vc functions from diff mode might be a bit
 > naughty.  Or perhaps not.
 ...


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: New commands for browsing diffs easily

[Lars Magne Ingebrigtsen]

Nick Roberts <nickrob@snap.net.nz> writes:

> Does log-view-mode do what you want?  Do `C-x v l' and then `d' on
> the appropriate line of the vc-change-log buffer (See VC status node
> in the manual).

I didn't know about the `d' command (thanks), but the usage is kinda
klunky what with the popping between the different buffers and how you
have to run the cursor around to get to the next/prev diff.

I'm mainly aiming for ease of use here -- scanning through a range of
diffs very easily (one keystroke) and quickly. 

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: New commands for browsing diffs easily

[Miles Bader]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
> I'm mainly aiming for ease of use here -- scanning through a range of
> diffs very easily (one keystroke) and quickly. 

It is pretty ugly to bind vc stuff in diff-mode though; isn't there a
vc-diff derived-mode or something that would be more appropriate?

Also the particular keybindings you used are probably not good --
note that when a diff-mode buffer is read-only, its bindings are usable
without the Meta prefix...

-Miles
-- 
"An atheist doesn't have to be someone who thinks he has a proof that there
can't be a god.  He only has to be someone who believes that the evidence
on the God question is at a similar level to the evidence on the werewolf
question."  [John McCarthy]

Re: New commands for browsing diffs easily

[Lars Magne Ingebrigtsen]

Miles Bader <miles.bader@necel.com> writes:

> It is pretty ugly to bind vc stuff in diff-mode though; isn't there a
> vc-diff derived-mode or something that would be more appropriate?

Hm...  is there such a beast?  I had a brief look around, but couldn't
really see anything obvious.

> Also the particular keybindings you used are probably not good --
> note that when a diff-mode buffer is read-only, its bindings are usable
> without the Meta prefix...

In which case the keys would be `C-n' and `C-p', which might not be
the best keys in the world to use for something like that.

`M-C->' and the like are pretty awkward, too.  Perhaps `M-u' and `M-d'
might be appropriate choices?  ("Up" and "down" the list of
revisions.) 

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: New commands for browsing diffs easily

[David Kastrup]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> `M-C->' and the like are pretty awkward, too.  Perhaps `M-u' and `M-d'
> might be appropriate choices?  ("Up" and "down" the list of
> revisions.) 

No.  "up" "down" in keybindings refers to hierarchy levels, not a
sequence.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: New commands for browsing diffs easily

[Lars Magne Ingebrigtsen]

David Kastrup <dak@gnu.org> writes:

> No.  "up" "down" in keybindings refers to hierarchy levels, not a
> sequence.

`M-f' and `M-b'.  (Forwards and backwards.)

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: New commands for browsing diffs easily

[Stefan Monnier]

>> I'm mainly aiming for ease of use here -- scanning through a range of
>> diffs very easily (one keystroke) and quickly. 

> It is pretty ugly to bind vc stuff in diff-mode though; isn't there a
> vc-diff derived-mode or something that would be more appropriate?

I think we can simply put the commands on C-x v <something> in VC's keymap
(which is active everywhere).


        Stefan

Re: New commands for browsing diffs easily

[Romain Francoise]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think we can simply put the commands on C-x v <something> in VC's
> keymap (which is active everywhere).

C-x v P and C-x v N?

-- 
Romain Francoise <romain@orebokech.com> | The sea! the sea! the open
it's a miracle -- http://orebokech.com/ | sea! The blue, the fresh, the
                                        | ever free! --Bryan W. Procter

Re: New commands for browsing diffs easily

[Lars Magne Ingebrigtsen]

Romain Francoise <romain@orebokech.com> writes:

> C-x v P and C-x v N?

Or lower-case p and v.

But these commands are only valid in (some) diff-mode buffers, so it
might be inappropriate to bind these globally.  On the other hand, the
other vc mode commands are also invalid in non-vc buffers, so it might
be a good idea anyway. 

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: New commands for browsing diffs easily

[Stefan Monnier]

> Romain Francoise <romain@orebokech.com> writes:
>> C-x v P and C-x v N?

> Or lower-case p and v.

C-x v v already exists.

> But these commands are only valid in (some) diff-mode buffers, so it
> might be inappropriate to bind these globally.  On the other hand, the
> other vc mode commands are also invalid in non-vc buffers, so it might
> be a good idea anyway.

And the C-x v p command could do something "useful" even in
(some) non-vc-diff buffers.


        Stefan

Re: New commands for browsing diffs easily

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> C-x v v already exists.

I meant `C-x v n'.  :-)

However, these aren't really very handy keystrokes.  The behind the
whole thing was to have a very convenient way to browse the story.

`C-x v n' isn't something you can lean on.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: New commands for browsing diffs easily

[Romain Francoise]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Or lower-case p and v.

The upper-case versions have the merit of being vaguely similar to what
P and N do in VC Annotate buffers.

-- 
Romain Francoise <romain@orebokech.com> | The sea! the sea! the open
it's a miracle -- http://orebokech.com/ | sea! The blue, the fresh, the
                                        | ever free! --Bryan W. Procter

Re: New commands for browsing diffs easily

[Ted Zlatanov]

On 11 Apr 2006, larsi@gnus.org wrote:

> I use `C-x v =' a lot, and one thing I've missed for ages is the
> ability to just walk through the diff history.  That is, after seeing
> that the diff in question wasn't really the one I was looking for, I
> just want to see the diffs that comes before or after.
>
> So this diff adds that.  I'm not quite sure that this is the best
> implementation -- calling vc functions from diff mode might be a bit
> naughty.  Or perhaps not.
>
> I bound the commands to `M-C-n' and `M-C-p' as all the more likely key
> strokes were already taken.

I think next-error and previous-error (used in occur-mode, compilation
modes, grep, and others right now) would be perfect.  Look at the
definition of next-error in simple.el, you just need to bind
next-error-function.

The nice thing is that you don't need new bindings, next-error and
previous-error will just DTRT in whatever mode you are.

I tried to suggest alternate names for the scary-sounding *-error
functions a while ago; right now you have goto-next-locus and
next-match.  previous-error is just next-error with a negative
argument, so there are no aliases for it as of now.  There's also
first-error to jump to the first item in the buffer.

Let me know if this is useful :)  It may not map directly to the way
you were thinking of diff navigation.

Ted

Re: New commands for browsing diffs easily

[Stefan Monnier]

> I think next-error and previous-error (used in occur-mode, compilation
> modes, grep, and others right now) would be perfect.  Look at the
> definition of next-error in simple.el, you just need to bind
> next-error-function.

They're alredy used to jump from hukn to hunk in the source buffer.


        Stefan

Re: New commands for browsing diffs easily

[Ted Zlatanov]

On 11 Apr 2006, monnier@iro.umontreal.ca wrote:

>> I think next-error and previous-error (used in occur-mode, compilation
>> modes, grep, and others right now) would be perfect.  Look at the
>> definition of next-error in simple.el, you just need to bind
>> next-error-function.
>
> They're alredy used to jump from hukn to hunk in the source buffer.

Oops :)  Sorry I was not thinking!

I've given thought to this actually, despite being obtuse above...
Where next-error and previous-error deal with "go to the next/previous
item" it might make sense to have a "meta-next" and "meta-previous"
which go to the next/previous container of items.  In Lars' example,
it would be a set of diffs for example, as he specified.  In
compilation mode, perhaps it would go to the next bunch of errors
relevant to the current compilation but in a different file.  Kind of
like next-paragraph, if next-error is like next-line.

Ted

Re: New commands for browsing diffs easily

[Stefan Monnier]

> I've given thought to this actually, despite being obtuse above...
> Where next-error and previous-error deal with "go to the next/previous
> item" it might make sense to have a "meta-next" and "meta-previous"
> which go to the next/previous container of items.  In Lars' example,
> it would be a set of diffs for example, as he specified.  In
> compilation mode, perhaps it would go to the next bunch of errors
> relevant to the current compilation but in a different file.  Kind of
> like next-paragraph, if next-error is like next-line.

Might be a good idea, but not for the feature being discussed: in diff-mode,
such a "next file error" command would naturally map to diff-file-next.


        Stefan

meta-{next, previous}-error (was: New commands for browsing diffs easily)

[Ted Zlatanov]

On 11 Apr 2006, monnier@iro.umontreal.ca wrote:

> Might be a good idea, but not for the feature being discussed: in diff-mode,
> such a "next file error" command would naturally map to diff-file-next.

I see.  Lars' code is really not linear in the next/previous chain but
more of a up/down (orthogonal to next-error/previous-error, I guess).
It doesn't map the way I thought it would.

I propose:

Level Command             Implemented?

-M    meta-previous-error N (e.g. diff-file-prev)
-1    previous-error      Y
+1    next-error          Y
+M    meta-next-error     N (e.g. diff-file-next)

(remember that *-error map to *-locus and *-match, so it's not only
about errors)

Any opinions?

Ted

Re: meta-{next, previous}-error

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> Level Command             Implemented?
>
> -M    meta-previous-error N (e.g. diff-file-prev)
> -1    previous-error      Y
> +1    next-error          Y
> +M    meta-next-error     N (e.g. diff-file-next)
>
> (remember that *-error map to *-locus and *-match, so it's not only
> about errors)

It makes sense to me.

Now, `next-error' and `previous-error' are bound to `M-g n' and `M-g
p', as well as `M-g M-n' and `M-g M-p', so how about `M-g M-f' and
`M-g M-b' for the "higher level" commands?  (Forwards, backward.)
`M-g M-f' is quote easy to type...

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: meta-{next, previous}-error

[Ted Zlatanov]

On 12 Apr 2006, larsi@quimbies.gnus.org wrote:

> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>> Level Command             Implemented?
>>
>> -M    meta-previous-error N (e.g. diff-file-prev)
>> -1    previous-error      Y
>> +1    next-error          Y
>> +M    meta-next-error     N (e.g. diff-file-next)
>>
>> (remember that *-error map to *-locus and *-match, so it's not only
>> about errors)
>
> It makes sense to me.
>
> Now, `next-error' and `previous-error' are bound to `M-g n' and `M-g
> p', as well as `M-g M-n' and `M-g M-p', so how about `M-g M-f' and
> `M-g M-b' for the "higher level" commands?  (Forwards, backward.)
> `M-g M-f' is quote easy to type...

Sure.  Any other comments before I produce a patch for simple.el?  It
will work like next-error: if the function is bound in the buffer it
will get called, so the mode just needs to bind the symbol to the
appropiate function.  I'll do the diff-mode example Stefan showed.

Thanks
Ted

Re: meta-{next, previous}-error

[Romain Francoise]

Ted Zlatanov <tzz@lifelogs.com> writes:

> Any other comments before I produce a patch for simple.el?

I think we should implement this new mechanism after the release, not
now.

-- 
Romain Francoise <romain@orebokech.com> | The sea! the sea! the open
it's a miracle -- http://orebokech.com/ | sea! The blue, the fresh, the
                                        | ever free! --Bryan W. Procter

Re: meta-{next, previous}-error

[Richard Stallman]

    > Any other comments before I produce a patch for simple.el?

    I think we should implement this new mechanism after the release, not
    now.

I agree.  This is a new feature, and not necessary now.

Re: meta-{next, previous}-error

[Ted Zlatanov]

On 14 Apr 2006, rms@gnu.org wrote:

> I agree.  This is a new feature, and not necessary now.

OK, I'll come back with the patch after the release.  Thanks for the
feedback!

Ted