# What's in git.git

6 messages from 2006-01-25 to 2006-01-26. Participants: Junio C Hamano, Jason Riedy, Santi Bejar.
Thread: https://gitlist.dev/t/3147

## Junio C Hamano, 2006-01-25 13:00

Subject: What's in git.git
Message-ID: <7vy814qx6o.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7vy814qx6o.fsf%40assigned-by-dhcp.cox.net

```
I've pushed out the following changes to "master" branch:

 * git-clone

   . --naked option is still accepted but is deprecated.  Please
     say --bare.

   . a repository cloned with --bare does not get "origin"
     branch, nor remotes/origin file.

   . Earlier we accepted more than one -o options, without
     complaining.  We now complain.

 * fetch and peek/ls-remote (Michal Ostrowski)

   . --upload-pack option to the underlying git-peek-remote and
     friends can be passed from the barebone Porcelain.

 * local push/pull environment fix (Matt Draisey)

   . We now clean "GIT_DIR" and friends from the environment
     when spawning a program on the other end to drive git
     native protocols on a local machine.  This uses unsetenv(),
     which is not strictly portable, but I was too lazy to fix
     it myself.  I am hoping that Jason Riedy will scream and
     give us a patch to make it work again on his Solaris box
     ;-).

 * sample update-hook rewrite (Andreas Ericsson)

 * asciidoc --unsafe workaround (Pavel Roskin)

In the "pu" branch, there are some interesting changes.  The
most immediately visible usability enhancement is the "combined
diff" option to git-diff-tree command.  Interested people can
try something like this:

	$ git checkout pu
	$ make clean strip all install
	$ git checkout master

Do not forget to come back to your "master" branch once you are
done, or your next pull would be screwed.

        $ git whatchanged --cc --abbrev pu

This would show an improved version of "git whatchanged -m -p".
The difference is that it uses the "dense combined diff"
suggested by Linus yesterday.  Commits near the tip of "pu" tend
to be Octopus, and you would see quite interesting combined
diffs for them.  I am not proud of the implementation itself,
and I am sure there are more bugs to be discovered, but overall
I am reasonably happy with what it shows.  Once it stabilizes,
it would be a good addition to gitweb UI.  In addition to the
"commitdiff" next to each parent, there would be an extra link
in a merge commit to get a combined diff.

The "annotate helper" change by Linus and "bound commit"
subproject support experiments are there as before.

There are other minor fixes and enhancements.  Not all of them
may make it to "master":

 * "diff-tree --abbrev --pretty" was still showing full 40
   characters of commit object names on "Merge:" line.

 * "rev-parse --abbrev[=<n>] $rev" is similar to
   "rev-parse --verify $rev", except that it shows an
   abbreviated object name.

 * The flag argument was handled confusingly by "rev-parse
   [--flags|--no-flags]".  The command now treats anything that
   come after --flags/--no-flags things to be parsed, not an
   option to control what the command does.  This was primarily
   needed to allow parsing out "--abbrev" option and giving it
   to "diff-tree" inside whatchanged.  All existing users have
   been adjusted to this change.  I do not think any of Cogito,
   gitk, nor stgit is affected.

```

## Junio C Hamano, 2006-01-25 19:24

Subject: Re: What's in git.git
Message-ID: <7v8xt4qffg.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7v8xt4qffg.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <8aa486160601250741k120f0021h@mail.gmail.com>

```
Santi Bejar <sbejar@gmail.com> writes:

> I think we should be consistent with respect the leading dots. Now the
> diff-tree line has the dots, but the merge line do not.

This is deliberate.  Merge lines do not have to align with
anything else for readability.

```

## Jason Riedy, 2006-01-25 20:36

Subject: Re: What's in git.git
Message-ID: <10786.1138221407@lotus.CS.Berkeley.EDU>
URL: https://gitlist.dev/e/10786.1138221407%40lotus.CS.Berkeley.EDU
In-Reply-To: <7vy814qx6o.fsf@assigned-by-dhcp.cox.net>

```
And Junio C Hamano writes:
 -      This uses unsetenv(), which is not strictly portable, but 
 - 	I was too lazy to fix it myself.

And you manage to catch me on the one day in the last month
I've played with git code...  Patch to add compat/unsetenv.c
coming shortly.  Passes unit tests and make test as well as 
before (I have some wierd, local-only cpio problems), but I 
haven't used this extensively.

People _with_ unsetenv can still add compat functions, you 
know.  ;)

Jason

```

## Junio C Hamano, 2006-01-25 23:56

Subject: Re: What's in git.git
Message-ID: <7vvew7lv4s.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7vvew7lv4s.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <8aa486160601250634v294857e0j@mail.gmail.com>

```
Santi Bejar <sbejar@gmail.com> writes:

> Junio C Hamano <junkio@cox.net> writes:
>...
>>         $ git whatchanged --cc --abbrev pu
>...
>       Then some comments :)
>
>       * There is an extra space between the +- and the content.

Thanks for noticing.  Fixed.

>       * I think the "diff command" could be just "diff --git".  And
>         "diff --git --cc" for the "dense combined diff".

I chose not say "--git" because I wanted to make sure people
would not run "git apply" by mistake.  I replaced them
with "--combined" and "--cc" to make the differences clearer.

>       * I miss the index lines

That would probably be easy to add in the later rounds if you
really want them, but I consider it lower priority for now.
This is designed to be human readable and not necessarily
machine applicable, so I do not see much point in showing them.

Mode changes, creation, deletion, rename, and copy may need to
be there, though.  The code currently punts on them.  Patches
welcome.

>       * In the case of 3 and more parents I find uninteresting the hunk
>         where the merge is equal to one (or more) of its children and
>         the rest are equal. For example the first hunk in 22573dd, where
>         all the children are equal expect the picked one:
>
> diff-tree 22573dd... (from parents)
> Merge: 92643a2 0359fe8 a428965 4aa079f
> Author: Junio C Hamano <junkio@cox.net>
> Date:   Wed Jan 25 03:51:37 2006 -0800
>
>     Merge lt/revlist,jc/diff,jc/bind
>
> diff --combined rev-list.c
> @@@@@ +37,7 @@@@@
>      static int dense = 1;
>      static int unpacked = 0;
>      static int bisect_list = 0;
> ---  static int tag_objects = 0;
> ---  static int tree_objects = 0;
> ---  static int blob_objects = 0;
> +++  static int list_objects = 0;
>      static int verbose_header = 0;
>      static int show_parents = 0;
>      static int hdr_termination = 0;

I noticed it and found it somewhat less interesting than others,
but left it the way.  It is "changed the same way from all of
these parents except this one", which is different from what I
culled in the version you are commenting on, i.e. "changed only
from one parent".

I've since updated the logic to drop these hunks as well.
Please take a look at the tip of "pu".

>       * Like you said in another thread, the line numbers of all the
>         files.

This is left as an exercise for the reader ;-)

> So, at the end, I suggest this output for the diff:
>...
> and this for the diff-raw:
>
> :100644 100644 56505b4... 538d21d... M  Makefile
> :100644 100644 30479b4... 538d21d... M  Makefile

I do not find this to be so interesting.  diff-raw is primarily
for quick sanity check and machine processing, so I'd rather not
play games when generating diff-raw in order to keep the latter
form of users sane.

```

## Junio C Hamano, 2006-01-26 12:12

Subject: Re: What's in git.git
Message-ID: <7vhd7rfare.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7vhd7rfare.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <8aa486160601260156h6157ca34s@mail.gmail.com>

```
Santi Bejar <sbejar@gmail.com> writes:

> 2006/1/26, Junio C Hamano <junkio@cox.net>:
>> No comment on your message yet, but please fix your MUA so that
>> your messages do not get rejected by the mailing list software.
>>
>> I suspect it is base64 CTE that causes it.
>>
>>         Content-Type: text/plain; charset=UTF-8
>>         Content-Transfer-Encoding: base64
>
> Sorry, about that. But I didn't get any notification.

You do not need to feel sorry.  It's just your voice is not
heard as widely as it should have been, and you did not know
about it.

The spam filter just drops things on the floor without bothering
to notify potential spammers.

I think this patch would address your two issues (applies on top
of "pu"), but I have not had time to test it seriously enough.

-- >8 --
[PATCH] combine-diff: better hunk splitting.

It considered an otherwise unchanged line that had line removals
in front of it an interesting line, which caused hunks to have
one extra the trailing context line.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 combine-diff.c |  142 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 116 insertions(+), 26 deletions(-)

0aa20bc0c276c3fcb7215ba9f591960178792261
diff --git a/combine-diff.c b/combine-diff.c
index 3b219a0..d94a93d 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -323,52 +323,142 @@ static unsigned long line_all_diff(struc
 	return different;
 }
 
-static int make_hunks(struct sline *sline, unsigned long cnt,
-		       int num_parent, int dense)
+static unsigned long adjust_hunk_tail(struct sline *sline,
+				      unsigned long all_mask,
+				      unsigned long hunk_begin,
+				      unsigned long i)
+{
+	/* i points at the first uninteresting line.
+	 * If the last line of the hunk was interesting
+	 * only because it has some deletion, then
+	 * it is not all that interesting for the
+	 * purpose of giving trailing context lines.
+	 */
+	if ((hunk_begin + 1 <= i) &&
+	    ((sline[i-1].flag & all_mask) == all_mask))
+		i--;
+	return i;
+}
+
+static unsigned long next_interesting(struct sline *sline,
+				      unsigned long mark,
+				      unsigned long i,
+				      unsigned long cnt,
+				      int uninteresting)
+{
+	while (i < cnt)
+		if (uninteresting ?
+		    !(sline[i].flag & mark) :
+		    (sline[i].flag & mark))
+			return i;
+		else
+			i++;
+	return cnt;
+}
+
+static int give_context(struct sline *sline, unsigned long cnt, int num_parent)
 {
 	unsigned long all_mask = (1UL<<num_parent) - 1;
 	unsigned long mark = (1UL<<num_parent);
 	unsigned long i;
 	int has_interesting = 0;
 
-	i = 0;
+	i = next_interesting(sline, mark, 0, cnt, 0);
+	if (cnt <= i)
+		return 0;
+
 	while (i < cnt) {
-		if (interesting(&sline[i], all_mask)) {
-			unsigned long j = (context < i) ? i - context : 0;
-			while (j <= i)
+		unsigned long j = (context < i) ? (i - context) : 0;
+		unsigned long k;
+		while (j < i)
+			sline[j++].flag |= mark;
+
+	again:
+		j = next_interesting(sline, mark, i, cnt, 1);
+		if (cnt <= j)
+			break; /* the rest are all interesting */
+
+		/* lookahead context lines */
+		k = next_interesting(sline, mark, j, cnt, 0);
+		j = adjust_hunk_tail(sline, all_mask, i, j);
+
+		if (k < j + context) {
+			/* k is interesting and [j,k) are not, but
+			 * paint them interesting because the gap is small.
+			 */
+			while (j < k)
 				sline[j++].flag |= mark;
-			while (++i < cnt) {
-				if (!interesting(&sline[i], all_mask))
-					break;
-				sline[i].flag |= mark;
-			}
-			j = (i + context < cnt) ? i + context : cnt;
-			while (i < j)
-				sline[i++].flag |= mark;
-			has_interesting = 1;
-			continue;
+			i = k;
+			goto again;
 		}
-		i++;
+
+		/* j is the first uninteresting line and there is
+		 * no overlap beyond it within context lines.
+		 */
+		i = k;
+		k = (j + context < cnt) ? j + context : cnt;
+		while (j < k)
+			sline[j++].flag |= mark;
+	}
+	return 1;
+}
+
+static int make_hunks(struct sline *sline, unsigned long cnt,
+		       int num_parent, int dense)
+{
+	unsigned long all_mask = (1UL<<num_parent) - 1;
+	unsigned long mark = (1UL<<num_parent);
+	unsigned long i;
+	int has_interesting = 0;
+
+	for (i = 0; i < cnt; i++) {
+		if (interesting(&sline[i], all_mask))
+			sline[i].flag |= mark;
+		else
+			sline[i].flag &= ~mark;
 	}
 	if (!dense)
-		return has_interesting;
+		return give_context(sline, cnt, num_parent);
 
 	/* Look at each hunk, and if we have changes from only one
 	 * parent, or the changes are the same from all but one
 	 * parent, mark that uninteresting.
 	 */
-	has_interesting = 0;
 	i = 0;
 	while (i < cnt) {
-		int j, hunk_end, same, diff;
+		unsigned long j, hunk_begin, hunk_end;
+		int same, diff;
 		unsigned long same_diff, all_diff;
 		while (i < cnt && !(sline[i].flag & mark))
 			i++;
 		if (cnt <= i)
 			break; /* No more interesting hunks */
-		for (hunk_end = i + 1; hunk_end < cnt; hunk_end++)
-			if (!(sline[hunk_end].flag & mark))
-				break;
+		hunk_begin = i;
+		for (j = i + 1; j < cnt; j++) {
+			if (!(sline[j].flag & mark)) {
+				/* Look beyond the end to see if there
+				 * is an interesting line after this
+				 * hunk within context span.
+				 */
+				unsigned long la; /* lookahead */
+				int contin = 0;
+				la = adjust_hunk_tail(sline, all_mask,
+						     hunk_begin, j);
+				la = (la + context < cnt) ?
+					(la + context) : cnt;
+				while (j <= --la) {
+					if (sline[la].flag & mark) {
+						contin = 1;
+						break;
+					}
+				}
+				if (!contin)
+					break;
+				j = la;
+			}
+		}
+		hunk_end = j;
+
 		/* [i..hunk_end) are interesting.  Now does it have
 		 * the same change with all but one parent?
 		 */
@@ -387,13 +477,13 @@ static int make_hunks(struct sline *slin
 		}
 		if ((num_parent - 1 <= same) || (diff == 1)) {
 			/* This hunk is not that interesting after all */
-			for (j = i; j < hunk_end; j++)
+			for (j = hunk_begin; j < hunk_end; j++)
 				sline[j].flag &= ~mark;
 		}
-		else
-			has_interesting = 1;
 		i = hunk_end;
 	}
+
+	has_interesting = give_context(sline, cnt, num_parent);
 	return has_interesting;
 }
 
-- 
1.1.4.g2cff

```

## Santi Bejar, 2006-01-26 16:24

Subject: Re: What's in git.git
Message-ID: <8aa486160601260824g72b127ddn@mail.gmail.com>
URL: https://gitlist.dev/e/8aa486160601260824g72b127ddn%40mail.gmail.com
In-Reply-To: <7vhd7rfare.fsf@assigned-by-dhcp.cox.net>

```
> I think this patch would address your two issues (applies on top
> of "pu"), but I have not had time to test it seriously enough.

It works here, thanks.

Just a little issue. With grafted commits the combined diff works but
the "Merge:" line is not shown.

Santi

```