threads / discuss / 43536

Re: git: how to produce context diffs?

Subject: Re: git: how to produce context diffs?

## tl;dr

8 messages between Nov 27, 2006 and Nov 27, 2006.

replies: 7people: 5as markdown or json

Bruno Haible· Nov 27, 2006, 14:16 UTC · lore

git: how to produce context diffs?

Hi,
Is this a bug in git-diff? The git-diff-files.html says:
  " When the environment variable GIT_EXTERNAL_DIFF is not set ...
    For example, if you prefer context diff:
    GIT_DIFF_OPTS=-c git-diff-index -p HEAD  "
This doesn't work for me with git-1.4.4:

$ unset GIT_EXTERNAL_DIFF $ export GIT_DIFF_OPTS=-c $ git-diff-index -p HEAD

diff --git a/configure.ac b/configure.ac
index 74901dc..d222ded 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 dnl Process this file with autoconf to produce a configure script.
-AC_INIT(hello, 2.1.1, bug-gnu-hello@gnu.org)
+AC_INIT(hello, 2.1.2, bug-gnu-hello@gnu.org)
 AC_CONFIG_SRCDIR([src/hello.c])
 
 AC_PREREQ(2.52)

Expected output:

$ git-diff-index -p HEAD
index 74901dc..d222ded 100644
diff --git a/configure.ac b/configure.ac
*** a/configure.ac
--- b/configure.ac
***************
*** 1,5 ****
  dnl Process this file with autoconf to produce a configure script.
! AC_INIT(hello, 2.1.1, bug-gnu-hello@gnu.org)
  AC_CONFIG_SRCDIR([src/hello.c])
  
  AC_PREREQ(2.52)
--- 1,5 ----
  dnl Process this file with autoconf to produce a configure script.
! AC_INIT(hello, 2.1.2, bug-gnu-hello@gnu.org)
  AC_CONFIG_SRCDIR([src/hello.c])
  
  AC_PREREQ(2.52)


(Really, while I find -u diffs fine for tiny changes, I find them unreadable
for rewrites of larger blocks, and cannot live without -c for these.)

When I look at diff.c around
       const char *diffopts = getenv("GIT_DIFF_OPTS");
it appears that only unified diffs are supported??
Jakub Narebski· Nov 27, 2006, 14:27 UTC · re: Bruno Haible · lore
Bruno Haible wrote:
Show 7 quoted lines
> Is this a bug in git-diff? The git-diff-files.html says:
> 
>   " When the environment variable GIT_EXTERNAL_DIFF is not set ...
>     For example, if you prefer context diff:
>     GIT_DIFF_OPTS=-c git-diff-index -p HEAD  "
> 
> This doesn't work for me with git-1.4.4:

Yes, the bug in documentation, I think. There is an option '-c' to git-diff, but it means "combined diff" (for merges), not "context diff".

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Sean· Nov 27, 2006, 15:19 UTC · re: Jakub Narebski · lore

On Mon, 27 Nov 2006 15:27:20 +0100 Jakub Narebski <jnareb@gmail.com> wrote:

Show 12 quoted lines
> Bruno Haible wrote:
> 
> > Is this a bug in git-diff? The git-diff-files.html says:
> > 
> >   " When the environment variable GIT_EXTERNAL_DIFF is not set ...
> >     For example, if you prefer context diff:
> >     GIT_DIFF_OPTS=-c git-diff-index -p HEAD  "
> > 
> > This doesn't work for me with git-1.4.4:
> 
> Yes, the bug in documentation, I think. There is an option '-c' to git-diff,
> but it means "combined diff" (for merges), not "context diff".
Indeed.  That documentation predates built-in diff completely.

It appears the only valid options now are "-u XX" and "--unified=XX". These options are never passed to diff, but rather used to control the internal diff. Strangely, it appears that gitk is even passing incorrect parameters via GIT_DIFF_OPTS.

Unless i've really missed something, the above documentation should be reworked to remove mention of running diff altogether, and should mention that the GIT_DIFF_OPTS only has two valid settings.

Jakub Narebski· Nov 27, 2006, 16:05 UTC · re: Sean · lore
Sean wrote:
Show 19 quoted lines
> On Mon, 27 Nov 2006 15:27:20 +0100
> Jakub Narebski <jnareb@gmail.com> wrote:
> 
>> Bruno Haible wrote:
>> 
>>> Is this a bug in git-diff? The git-diff-files.html says:
>>> 
>>>   " When the environment variable GIT_EXTERNAL_DIFF is not set ...
>>>     For example, if you prefer context diff:
>>>     GIT_DIFF_OPTS=-c git-diff-index -p HEAD  "
>>> 
>>> This doesn't work for me with git-1.4.4:
>> 
>> Yes, the bug in documentation, I think. There is an option '-c' to git-diff,
>> but it means "combined diff" (for merges), not "context diff".
> 
> Indeed.  That documentation predates built-in diff completely.
> 
> It appears the only valid options now are "-u XX" and "--unified=XX".
Which both mean the same.
> These options are never passed to diff, but rather used to control
> the internal diff.  Strangely, it appears that gitk is even passing
> incorrect parameters via GIT_DIFF_OPTS.
Which, in convoluted way is said in documentation (the fact that
GIT_DIFF_OPTS affect internal diff).
 
> Unless i've really missed something, the above documentation should be
> reworked to remove mention of running diff altogether, and should mention
> that the GIT_DIFF_OPTS only has two valid settings.
For now.
-- 
Jakub Narebski
Sean· Nov 27, 2006, 17:06 UTC · re: Jakub Narebski · lore

On Mon, 27 Nov 2006 17:05:35 +0100 Jakub Narebski <jnareb@gmail.com> wrote:

> > It appears the only valid options now are "-u XX" and "--unified=XX".
> 
> Which both mean the same.

Yes, I guess I should have said that. The point was really that you can't pass any-diff-option and expect it to work.

> Which, in convoluted way is said in documentation (the fact that
> GIT_DIFF_OPTS affect internal diff).

The idea is to make it less convoluted. It directly says that -c should work _because_ it use to work when the options were passed to external diff. It's not just -c that won't work, none of the other myriad external diff options will work either.

Show 5 quoted lines
> > Unless i've really missed something, the above documentation should be
> > reworked to remove mention of running diff altogether, and should mention
> > that the GIT_DIFF_OPTS only has two valid settings.
> 
> For now.

Well the documentation can be updated later if additional options are added.

Sean· Nov 27, 2006, 15:38 UTC · re: Jakub Narebski · lore

On Mon, 27 Nov 2006 15:27:20 +0100 Jakub Narebski <jnareb@gmail.com> wrote:

Show 12 quoted lines
> Bruno Haible wrote:
> 
> > Is this a bug in git-diff? The git-diff-files.html says:
> > 
> >   " When the environment variable GIT_EXTERNAL_DIFF is not set ...
> >     For example, if you prefer context diff:
> >     GIT_DIFF_OPTS=-c git-diff-index -p HEAD  "
> > 
> > This doesn't work for me with git-1.4.4:
> 
> Yes, the bug in documentation, I think. There is an option '-c' to git-diff,
> but it means "combined diff" (for merges), not "context diff".
Indeed.  That documentation predates built-in diff completely.

It appears the only valid options now are "-u XX" and "--unified=XX". These options are never passed to diff, but rather used to control the internal diff. Strangely, it appears that gitk is even passing incorrect parameters via GIT_DIFF_OPTS.

Unless i've really missed something, the above documentation should be reworked to remove mention of running diff altogether, and should mention that the GIT_DIFF_OPTS only has two valid settings.

Thomas Kolejka· Nov 27, 2006, 18:33 UTC · re: Bruno Haible · lore
Hello,
I'm using the following script 'external-diff.sh':

-- # path old-file old-hex old-mode new-file new-hex new-mode # $1 $2 $3 $4 $5 $6 $7

old_hex=$3 new_hex=`git-hash-object $5`

if [ "$old_hex" = "$new_hex" ]
then
	exit 0
fi

echo "diff --git a/$1 b/$1" echo "index ${old_hex:0:7}..${new_hex:0:7} $4"

diff -L a/$1 -L b/$1 -pc $2 $5

exit 0 --

which can be called by:

export GIT_EXTERNAL_DIFF=external-diff.sh git-diff-index master -p > my.patch

Bye, Thomas Kolejka

-- 
"Ein Herz für Kinder" - Ihre Spende hilft! Aktion: www.deutschlandsegelt.de
Unser Dankeschön: Ihr Name auf dem Segel der 1. deutschen America's C
Junio C Hamano· Nov 27, 2006, 19:18 UTC · re: Bruno Haible · lore
There are a few things to note:
 * Obviously diff.c::builtin_diff() needs to be taught to
   generate the new format, both in the patch text and also in
   the patch and hunk header part.
 * Corresponding changes to builtin-apply.c need to be made;
   most of the functions in the callchain that starts at
   apply_patch() down to apply_one_fragment() need to be taught
   about the copied context format.  Also code to count diffstat
   in builtin-apply.c needs to be adjusted.  This part should be
   done first before diff.c if we were to support copied context
   diff, because we should not be generating what we cannot
   apply.
 * I do not think of a sane way to show a combined diff from
   multiple preimages in copied context format; combine-diff.c
   can probably be left as it is.

I too find larger hunks easier to read with copied context than with unified context, but what needs to be done listed above is a nontrivial amount of work. Since the beginning, git has only supported unified context format and not copied context format, and apparently people who lived with git for the last 18 months have survived without copied context format, so it is of very low priority even for me right now.

I am not opposed to adding the copied context format as long as somebody else does the work ;-).

← back to recent threads