[PATCH] git.el: Diff only file at point by default

[PATCH] git.el: Diff only file at point by default

[David Kågedal]

Use prefix (C-u) to diff all marked files instead.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
 contrib/emacs/git.el |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

For anyone who has used pcvs (or dsvn) this makes much more sense. The
typical usage scenario is that you want to mark the files that are
relevant, and you do it incrementally by checking if file A is
relevant and has no strange changes by pressing = and then marking it,
proceeding to check file B by pressing = on it and potentially marking
it as well and so on.

The current way that = works means that you have to check each file
first and remember which one you want to mark. Or diff everything and
then read through the whole diff and find match the output against the
files to mark. It is possible to do it like that as well, but it is
much more convenient to do it as described above, and there is a
strong precedent in emacs frontends to make the diff command only diff
the current file by default.

The old behaviour can be restored by using C-u =.

diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index c30d20a..a3477a4 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -772,6 +772,11 @@ Return the list of files that haven't been handled."
               (setq file (pop files))
             (setq node (ewoc-next status node))))))))
 
+(defun git-current-file ()
+  "Return the file at point."
+  (unless git-status (error "Not in git-status buffer."))
+  (ewoc-data (ewoc-locate git-status)))
+
 (defun git-marked-files ()
   "Return a list of all marked files, or if none a list containing just the file at cursor position."
   (unless git-status (error "Not in git-status buffer."))
@@ -1138,10 +1143,10 @@ Return the list of files that haven't been handled."
   (when (eq (window-buffer) (current-buffer))
     (shrink-window-if-larger-than-buffer)))
 
-(defun git-diff-file ()
-  "Diff the marked file(s) against HEAD."
-  (interactive)
-  (let ((files (git-marked-files)))
+(defun git-diff-file (arg)
+  "Diff the marked file(s) against HEAD, or the marked files if a prefix arg is given."
+  (interactive "P")
+  (let ((files (if arg (git-marked-files) (list (git-current-file)))))
     (git-setup-diff-buffer
      (apply #'git-run-command-buffer "*git-diff*" "diff-index" "-p" "-M" "HEAD" "--" (git-get-filenames files)))))
 
-- 
1.6.0.rc2.7.gbf8a


-- 
David Kågedal

Re: [PATCH] git.el: Diff only file at point by default

[Junio C Hamano]

David Kågedal <davidk@lysator.liu.se> writes:

> Use prefix (C-u) to diff all marked files instead.
>
> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
> ---
>  contrib/emacs/git.el |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
>
> For anyone who has used pcvs (or dsvn) this makes much more sense. The

Looks sensible to me; Alexandre?

> typical usage scenario is that you want to mark the files that are
> relevant, and you do it incrementally by checking if file A is
> relevant and has no strange changes by pressing = and then marking it,
> proceeding to check file B by pressing = on it and potentially marking
> it as well and so on.
>
> The current way that = works means that you have to check each file
> first and remember which one you want to mark. Or diff everything and
> then read through the whole diff and find match the output against the
> files to mark. It is possible to do it like that as well, but it is
> much more convenient to do it as described above, and there is a
> strong precedent in emacs frontends to make the diff command only diff
> the current file by default.
>
> The old behaviour can be restored by using C-u =.
>
> diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
> index c30d20a..a3477a4 100644
> --- a/contrib/emacs/git.el
> +++ b/contrib/emacs/git.el
> @@ -772,6 +772,11 @@ Return the list of files that haven't been handled."
>                (setq file (pop files))
>              (setq node (ewoc-next status node))))))))
>  
> +(defun git-current-file ()
> +  "Return the file at point."
> +  (unless git-status (error "Not in git-status buffer."))
> +  (ewoc-data (ewoc-locate git-status)))
> +
>  (defun git-marked-files ()
>    "Return a list of all marked files, or if none a list containing just the file at cursor position."
>    (unless git-status (error "Not in git-status buffer."))
> @@ -1138,10 +1143,10 @@ Return the list of files that haven't been handled."
>    (when (eq (window-buffer) (current-buffer))
>      (shrink-window-if-larger-than-buffer)))
>  
> -(defun git-diff-file ()
> -  "Diff the marked file(s) against HEAD."
> -  (interactive)
> -  (let ((files (git-marked-files)))
> +(defun git-diff-file (arg)
> +  "Diff the marked file(s) against HEAD, or the marked files if a prefix arg is given."
> +  (interactive "P")
> +  (let ((files (if arg (git-marked-files) (list (git-current-file)))))
>      (git-setup-diff-buffer
>       (apply #'git-run-command-buffer "*git-diff*" "diff-index" "-p" "-M" "HEAD" "--" (git-get-filenames files)))))
>  
> -- 
> 1.6.0.rc2.7.gbf8a
>
>
> -- 
> David Kågedal
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git.el: Diff only file at point by default

[Alexandre Julliard]

Junio C Hamano <gitster@pobox.com> writes:

> David Kågedal <davidk@lysator.liu.se> writes:
>
>> Use prefix (C-u) to diff all marked files instead.
>>
>> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
>> ---
>>  contrib/emacs/git.el |   13 +++++++++----
>>  1 files changed, 9 insertions(+), 4 deletions(-)
>>
>> For anyone who has used pcvs (or dsvn) this makes much more sense. The
>
> Looks sensible to me; Alexandre?

Looks OK to me, but I think you should apply the same treatment to all
the other diff functions. Also the documentation string needs a bit more
work.

-- 
Alexandre Julliard
julliard@winehq.org

Re: [PATCH] git.el: Diff only file at point by default

[David Kågedal]

Alexandre Julliard <julliard@winehq.org> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> David Kågedal <davidk@lysator.liu.se> writes:
>>
>>> Use prefix (C-u) to diff all marked files instead.
>>>
>>> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
>>> ---
>>>  contrib/emacs/git.el |   13 +++++++++----
>>>  1 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> For anyone who has used pcvs (or dsvn) this makes much more sense. The
>>
>> Looks sensible to me; Alexandre?
>
> Looks OK to me, but I think you should apply the same treatment to all
> the other diff functions. Also the documentation string needs a bit more
> work.

It's the only one I use, so I didn't think about any other diff
commands. But I'll have a look.

-- 
David Kågedal

[PATCH] git.el: Diff only file at point by default

[David Kågedal]

Use prefix (C-u) to diff all marked files.

Signed-off-by: David Kågedal <davidk@lysator.liu.se>
---
 contrib/emacs/git.el |   62 +++++++++++++++++++++++++++++---------------------
 1 files changed, 36 insertions(+), 26 deletions(-)

Here is an updated patch that udpate the stage diff commands as
well. It doesn't touch git-diff-file-merge-head since that already
uses prefix arguments. Don't know if there is a solution to that.

diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
index c1cf1cb..de9d0f4 100644
--- a/contrib/emacs/git.el
+++ b/contrib/emacs/git.el
@@ -771,6 +771,11 @@ Return the list of files that haven't been handled."
               (setq file (pop files))
             (setq node (ewoc-next status node))))))))
 
+(defun git-current-file ()
+  "Return the file at point."
+  (unless git-status (error "Not in git-status buffer."))
+  (ewoc-data (ewoc-locate git-status)))
+
 (defun git-marked-files ()
   "Return a list of all marked files, or if none a list containing just the file at cursor position."
   (unless git-status (error "Not in git-status buffer."))
@@ -1137,10 +1142,11 @@ Return the list of files that haven't been handled."
   (when (eq (window-buffer) (current-buffer))
     (shrink-window-if-larger-than-buffer)))
 
-(defun git-diff-file ()
-  "Diff the marked file(s) against HEAD."
-  (interactive)
-  (let ((files (git-marked-files)))
+(defun git-diff-file (arg)
+  "Diff the current file against HEAD.
+With a prefix arg, diff the marked files instead."
+  (interactive "P")
+  (let ((files (if arg (git-marked-files) (list (git-current-file)))))
     (git-setup-diff-buffer
      (apply #'git-run-command-buffer "*git-diff*" "diff-index" "-p" "-M" "HEAD" "--" (git-get-filenames files)))))
 
@@ -1154,31 +1160,35 @@ Return the list of files that haven't been handled."
      (apply #'git-run-command-buffer "*git-diff*" "diff-index" "-p" "-M"
             (or (nth (1- arg) merge-heads) "HEAD") "--" (git-get-filenames files)))))
 
-(defun git-diff-unmerged-file (stage)
-  "Diff the marked unmerged file(s) against the specified stage."
-  (let ((files (git-marked-files)))
+(defun git-diff-unmerged-file (stage arg)
+  "Diff the files against the specified stage."
+  (let ((files (if arg (git-marked-files) (list (git-current-file)))))
     (git-setup-diff-buffer
      (apply #'git-run-command-buffer "*git-diff*" "diff-files" "-p" stage "--" (git-get-filenames files)))))
 
-(defun git-diff-file-base ()
-  "Diff the marked unmerged file(s) against the common base file."
-  (interactive)
-  (git-diff-unmerged-file "-1"))
-
-(defun git-diff-file-mine ()
-  "Diff the marked unmerged file(s) against my pre-merge version."
-  (interactive)
-  (git-diff-unmerged-file "-2"))
-
-(defun git-diff-file-other ()
-  "Diff the marked unmerged file(s) against the other's pre-merge version."
-  (interactive)
-  (git-diff-unmerged-file "-3"))
-
-(defun git-diff-file-combined ()
-  "Do a combined diff of the marked unmerged file(s)."
-  (interactive)
-  (git-diff-unmerged-file "-c"))
+(defun git-diff-file-base (arg)
+  "Diff the current file against the common base file.
+With a prefix arg, diff the marked files instead."
+  (interactive "P")
+  (git-diff-unmerged-file "-1" arg))
+
+(defun git-diff-file-mine (arg)
+  "Diff the current file against my pre-merge version.
+With a prefix arg, diff the marked files instead."
+  (interactive "P")
+  (git-diff-unmerged-file "-2" arg))
+
+(defun git-diff-file-other (arg)
+  "Diff the current file against the other's pre-merge version.
+With a prefix arg, diff the marked files instead."
+  (interactive "P")
+  (git-diff-unmerged-file "-3" arg))
+
+(defun git-diff-file-combined (arg)
+  "Do a combined diff of the marked unmerged file(s).
+With a prefix arg, diff the marked files instead."
+  (interactive "P")
+  (git-diff-unmerged-file "-c" arg))
 
 (defun git-diff-file-idiff ()
   "Perform an interactive diff on the current file."
-- 
1.6.0.rc2.7.gbf8a


-- 
David Kågedal

Re: [PATCH] git.el: Diff only file at point by default

[Junio C Hamano]

David Kågedal <davidk@lysator.liu.se> writes:

> Use prefix (C-u) to diff all marked files.
>
> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
> ---
>  contrib/emacs/git.el |   62 +++++++++++++++++++++++++++++---------------------
>  1 files changed, 36 insertions(+), 26 deletions(-)
>
> Here is an updated patch that udpate the stage diff commands as
> well. It doesn't touch git-diff-file-merge-head since that already
> uses prefix arguments. Don't know if there is a solution to that.

Yeah, me neither.  But otherwise looks quite straightforward conversion to
me.  Alexandre?

> diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
> index c1cf1cb..de9d0f4 100644
> --- a/contrib/emacs/git.el
> +++ b/contrib/emacs/git.el
> @@ -771,6 +771,11 @@ Return the list of files that haven't been handled."
>                (setq file (pop files))
>              (setq node (ewoc-next status node))))))))
>  
> +(defun git-current-file ()
> +  "Return the file at point."
> +  (unless git-status (error "Not in git-status buffer."))
> +  (ewoc-data (ewoc-locate git-status)))
> +
>  (defun git-marked-files ()
>    "Return a list of all marked files, or if none a list containing just the file at cursor position."
>    (unless git-status (error "Not in git-status buffer."))
> @@ -1137,10 +1142,11 @@ Return the list of files that haven't been handled."
>    (when (eq (window-buffer) (current-buffer))
>      (shrink-window-if-larger-than-buffer)))
>  
> -(defun git-diff-file ()
> -  "Diff the marked file(s) against HEAD."
> -  (interactive)
> -  (let ((files (git-marked-files)))
> +(defun git-diff-file (arg)
> +  "Diff the current file against HEAD.
> +With a prefix arg, diff the marked files instead."
> +  (interactive "P")
> +  (let ((files (if arg (git-marked-files) (list (git-current-file)))))
>      (git-setup-diff-buffer
>       (apply #'git-run-command-buffer "*git-diff*" "diff-index" "-p" "-M" "HEAD" "--" (git-get-filenames files)))))
>  
> @@ -1154,31 +1160,35 @@ Return the list of files that haven't been handled."
>       (apply #'git-run-command-buffer "*git-diff*" "diff-index" "-p" "-M"
>              (or (nth (1- arg) merge-heads) "HEAD") "--" (git-get-filenames files)))))
>  
> -(defun git-diff-unmerged-file (stage)
> -  "Diff the marked unmerged file(s) against the specified stage."
> -  (let ((files (git-marked-files)))
> +(defun git-diff-unmerged-file (stage arg)
> +  "Diff the files against the specified stage."
> +  (let ((files (if arg (git-marked-files) (list (git-current-file)))))
>      (git-setup-diff-buffer
>       (apply #'git-run-command-buffer "*git-diff*" "diff-files" "-p" stage "--" (git-get-filenames files)))))
>  
> -(defun git-diff-file-base ()
> -  "Diff the marked unmerged file(s) against the common base file."
> -  (interactive)
> -  (git-diff-unmerged-file "-1"))
> -
> -(defun git-diff-file-mine ()
> -  "Diff the marked unmerged file(s) against my pre-merge version."
> -  (interactive)
> -  (git-diff-unmerged-file "-2"))
> -
> -(defun git-diff-file-other ()
> -  "Diff the marked unmerged file(s) against the other's pre-merge version."
> -  (interactive)
> -  (git-diff-unmerged-file "-3"))
> -
> -(defun git-diff-file-combined ()
> -  "Do a combined diff of the marked unmerged file(s)."
> -  (interactive)
> -  (git-diff-unmerged-file "-c"))
> +(defun git-diff-file-base (arg)
> +  "Diff the current file against the common base file.
> +With a prefix arg, diff the marked files instead."
> +  (interactive "P")
> +  (git-diff-unmerged-file "-1" arg))
> +
> +(defun git-diff-file-mine (arg)
> +  "Diff the current file against my pre-merge version.
> +With a prefix arg, diff the marked files instead."
> +  (interactive "P")
> +  (git-diff-unmerged-file "-2" arg))
> +
> +(defun git-diff-file-other (arg)
> +  "Diff the current file against the other's pre-merge version.
> +With a prefix arg, diff the marked files instead."
> +  (interactive "P")
> +  (git-diff-unmerged-file "-3" arg))
> +
> +(defun git-diff-file-combined (arg)
> +  "Do a combined diff of the marked unmerged file(s).
> +With a prefix arg, diff the marked files instead."
> +  (interactive "P")
> +  (git-diff-unmerged-file "-c" arg))
>  
>  (defun git-diff-file-idiff ()
>    "Perform an interactive diff on the current file."
> -- 
> 1.6.0.rc2.7.gbf8a
>
>
> -- 
> David Kågedal

Re: [PATCH] git.el: Diff only file at point by default

[Alexandre Julliard]

David Kågedal <davidk@lysator.liu.se> writes:

> Use prefix (C-u) to diff all marked files.
>
> Signed-off-by: David Kågedal <davidk@lysator.liu.se>
> ---
>  contrib/emacs/git.el |   62 +++++++++++++++++++++++++++++---------------------
>  1 files changed, 36 insertions(+), 26 deletions(-)
>
> Here is an updated patch that udpate the stage diff commands as
> well. It doesn't touch git-diff-file-merge-head since that already
> uses prefix arguments. Don't know if there is a solution to that.

I'd suggest to change git-diff-file-merge-head to act on the current
file too. This means that the behavior is consistent across all diff
functions, even if we lose the ability to do git-diff-file-merge-head on
all marked files. Later on we may want to use a different mechanism than
the prefix arg to toggle the marked file behavior for all diff functions
(something like the pcl-cvs toggle-marks function).

Also make sure to check the callers of the diff functions you are
changing; at least git-log-edit-diff got broken by your change, there
may be others.

-- 
Alexandre Julliard
julliard@winehq.org