threads / patch / 15156

patchgit.el: Diff only file at point by default

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

## tl;dr

7 messages between Aug 22, 2008 and Aug 30, 2008. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

David Kågedal· Aug 22, 2008, 07:58 UTC · lore
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 =.
Show changes to contrib/emacs/git.el +9 −4
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
Junio C Hamano· Aug 24, 2008, 01:02 UTC · re: David Kågedal · lore

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

David Kågedal <davidk@lysator.liu.se> writes:
Show 8 quoted lines
> 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?
Show 57 quoted lines
> 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
Alexandre Julliard· Aug 25, 2008, 09:30 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 12 quoted lines
> 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
David Kågedal· Aug 25, 2008, 13:21 UTC · re: Alexandre Julliard · lore

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

Alexandre Julliard <julliard@winehq.org> writes:
Show 18 quoted lines
> 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
David Kågedal· Aug 26, 2008, 22:22 UTC · re: David Kågedal · lore
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.

Show changes to contrib/emacs/git.el +36 −26
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
Junio C Hamano· Aug 27, 2008, 19:36 UTC · re: David Kågedal · lore

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

David Kågedal <davidk@lysator.liu.se> writes:
Show 10 quoted lines
> 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?

Show 96 quoted lines
> 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
Alexandre Julliard· Aug 30, 2008, 18:29 UTC · re: David Kågedal · lore

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

David Kågedal <davidk@lysator.liu.se> writes:
Show 10 quoted lines
> 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

← back to recent threads