Re: Call Me Gitless

Daniel Barkalow [off-list ref] writes:

Actually, this weekend I was trying to cherry-pick the aggregated changes 
to certain files from one branch onto another, and was repeatedly confused 
by the fact that the only available diffs are backwards and there're no 
clues in the output. (That is, you can't get the difference between (---) 
the {index,working tree} and (+++) some commit, and when you've done "git 
diff messy", the resulting diff doesn't give any clues that you're 
deciding whether to add the - lines and remove the + lines.)

I do not know if I like the end result, but here is a patch to make the
traditional a/ and b/ prefix more mnemonic.

A lot of existing tests and documentation need to be updated, if we were
to do this, though.    The first test to fail is t1200-tutorial.sh.

Obviously not tested except for creating this patch that pretends to be a
format-patch output.  You can tell that I just did this only in the work
tree now.

-- >8 --
diff: vary default prefix depending on what are compared

This implements Daniel's idea to indicate what are compared by using
prefix different from the traditional a/ and b/ in the textual diff
header:

    "git diff" compares the (i)ndex and the (w)ork tree;
    "git diff HEAD" compares a (c)ommit and the (w)ork tree;
    "git diff --cached" compares a (c)ommit and the (i)ndex;
    "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.

Because these mnemonics now have meanings, they are swapped when reverse
diff is in effect.

Signed-off-by: Junio C Hamano <redacted>
---
 builtin-diff.c |    2 ++
 diff-lib.c     |    3 +++
 diff.c         |   38 +++++++++++++++++++++++++++++++-------
 diff.h         |    2 ++
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git i/builtin-diff.c w/builtin-diff.c
index 7ffea97..ecec753 100644
--- i/builtin-diff.c
+++ w/builtin-diff.c

@@ -74,6 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
 		die("'%s': not a regular file or symlink", path);
 
+	diff_set_default_prefix(&revs->diffopt, "o/", "w/");
+
 	if (blob[0].mode == S_IFINVALID)
 		blob[0].mode = canon_mode(st.st_mode);
 

diff --git i/diff-lib.c w/diff-lib.c
index e7eaff9..969f8c1 100644
--- i/diff-lib.c
+++ w/diff-lib.c

@@ -63,6 +63,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			      ? CE_MATCH_RACY_IS_DIRTY : 0);
 	char symcache[PATH_MAX];
 
+	diff_set_default_prefix(&revs->diffopt, "i/", "w/");
+
 	if (diff_unmerged_stage < 0)
 		diff_unmerged_stage = 2;
 	entries = active_nr;

@@ -469,6 +471,7 @@ int run_diff_index(struct rev_info *revs, int cached)
 	if (unpack_trees(1, &t, &opts))
 		exit(128);
 
+	diff_set_default_prefix(&revs->diffopt, "c/", cached ? "i/" : "w/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 	return 0;

diff --git i/diff.c w/diff.c
index bf5d5f1..1c518c6 100644
--- i/diff.c
+++ w/diff.c

@@ -305,6 +305,15 @@ static void emit_rewrite_diff(const char *name_a,
 	const char *new = diff_get_color(color_diff, DIFF_FILE_NEW);
 	const char *reset = diff_get_color(color_diff, DIFF_RESET);
 	static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
+	const char *a_prefix, *b_prefix;
+
+	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		a_prefix = o->b_prefix;
+		b_prefix = o->a_prefix;
+	} else {
+		a_prefix = o->a_prefix;
+		b_prefix = o->b_prefix;
+	}
 
 	name_a += (*name_a == '/');
 	name_b += (*name_b == '/');

@@ -313,8 +322,8 @@ static void emit_rewrite_diff(const char *name_a,
 
 	strbuf_reset(&a_name);
 	strbuf_reset(&b_name);
-	quote_two_c_style(&a_name, o->a_prefix, name_a, 0);
-	quote_two_c_style(&b_name, o->b_prefix, name_b, 0);
+	quote_two_c_style(&a_name, a_prefix, name_a, 0);
+	quote_two_c_style(&b_name, b_prefix, name_b, 0);
 
 	diff_populate_filespec(one, 0);
 	diff_populate_filespec(two, 0);

@@ -1424,6 +1433,14 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
 	return NULL;
 }
 
+void diff_set_default_prefix(struct diff_options *options, const char *a, const char *b)
+{
+	if (!options->a_prefix)
+		options->a_prefix = a;
+	if (!options->b_prefix)
+		options->b_prefix = b;
+}
+
 static void builtin_diff(const char *name_a,
 			 const char *name_b,
 			 struct diff_filespec *one,

@@ -1437,9 +1454,19 @@ static void builtin_diff(const char *name_a,
 	char *a_one, *b_two;
 	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
 	const char *reset = diff_get_color_opt(o, DIFF_RESET);
+	const char *a_prefix, *b_prefix;
 
-	a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
-	b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
+	diff_set_default_prefix(o, "a/", "b/");
+	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+		a_prefix = o->b_prefix;
+		b_prefix = o->a_prefix;
+	} else {
+		a_prefix = o->a_prefix;
+		b_prefix = o->b_prefix;
+	}
+
+	a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
+	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
 	fprintf(o->file, "%sdiff --git %s %s%s\n", set, a_one, b_two, reset);

@@ -2298,9 +2325,6 @@ void diff_setup(struct diff_options *options)
 	else
 		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
-
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
 }
 
 int diff_setup_done(struct diff_options *options)

diff --git i/diff.h w/diff.h
index 50fb5dd..5782fef 100644
--- i/diff.h
+++ w/diff.h

@@ -160,6 +160,8 @@ extern void diff_tree_combined(const unsigned char *sha1, const unsigned char pa
 
 extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *);
 
+void diff_set_default_prefix(struct diff_options *options, const char *a, const char *b);
+
 extern void diff_addremove(struct diff_options *,
 			   int addremove,
 			   unsigned mode,

Junio C Hamano wrote:

This implements Daniel's idea to indicate what are compared by using
prefix different from the traditional a/ and b/ in the textual diff
header:

    "git diff" compares the (i)ndex and the (w)ork tree;
    "git diff HEAD" compares a (c)ommit and the (w)ork tree;
    "git diff --cached" compares a (c)ommit and the (i)ndex;
    "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.

Because these mnemonics now have meanings, they are swapped when reverse
diff is in effect.

I like this proposal-ish; making the prefixes more intuitive could be
useful when looking at a bare diff from git too.  I'd put some time in
to help implement this.

-- 
Marcus Griep
GPG Key ID: 0x5E968152
——
http://www.boohaunt.net
את.ψο´

Junio C Hamano wrote:

I do not know if I like the end result, but here is a patch to make the
traditional a/ and b/ prefix more mnemonic.

diff: vary default prefix depending on what are compared

I consider this an improvement.
-- 
Sincerely,
           Stephen R. van den Berg.
"Papers in string theory are published at a rate above the speed of light.
 This is no problem since no information is being transmitted." -- H. Kleinert

Junio C Hamano [off-list ref] writes:

Daniel Barkalow [off-list ref] writes:

quoted

Actually, this weekend I was trying to cherry-pick the aggregated changes 
to certain files from one branch onto another, and was repeatedly confused 
by the fact that the only available diffs are backwards and there're no 
clues in the output. (That is, you can't get the difference between (---) 
the {index,working tree} and (+++) some commit, and when you've done "git 
diff messy", the resulting diff doesn't give any clues that you're 
deciding whether to add the - lines and remove the + lines.)

I do not know if I like the end result, but here is a patch to make the
traditional a/ and b/ prefix more mnemonic.

A lot of existing tests and documentation need to be updated, if we were
to do this, though.    The first test to fail is t1200-tutorial.sh.

Obviously not tested except for creating this patch that pretends to be a
format-patch output.  You can tell that I just did this only in the work
tree now.

-- >8 --
diff: vary default prefix depending on what are compared

This implements Daniel's idea to indicate what are compared by using
prefix different from the traditional a/ and b/ in the textual diff
header:

    "git diff" compares the (i)ndex and the (w)ork tree;
    "git diff HEAD" compares a (c)ommit and the (w)ork tree;
    "git diff --cached" compares a (c)ommit and the (i)ndex;
    "git diff HEAD:f /tmp/f" compares an (o)bject and (w)ork tree.

Because these mnemonics now have meanings, they are swapped when reverse
diff is in effect.

I was thinking about reusing estended SHA1 syntax in the form
of :0:a/file or ::a/file for index, a/file for working directory,
and HEAD:a/file for a tree version.  But your way is I think better;
of course if you remember mnemonics (and they are documented, aren't
they?).

BTW. I wonder why in above patch, which I guess is result of running
git-format-patch and should be between TWO TREES, doesn't use standard
'a/' and 'b/' (git-show should also use standard, default prefixes).

-- 
Jakub Narebski
Poland
ShadeHawk on #git

On Mon, Aug 18, 2008 at 08:22:19PM -0700, Junio C Hamano wrote:

I do not know if I like the end result, but here is a patch to make the
traditional a/ and b/ prefix more mnemonic.

Hmm. Something deep in my gut doesn't like this, just because I like the
fact that no matter how I prepare a diff (and I do tend to do it
different ways and post to the mailing list) it always ends up the same.
For example, I sometimes "hand-generate" patch messages meant to be
applied by git-am by doing a diff between the working tree and index and
pasting the result into an email. It just feels a bit wrong for it not
to be the exact output I would get from commiting and running
format-patch.

And yes, obviously the prefix should be thrown away by am (and any sane
tools), so it shouldn't matter. So I don't think there is a technical
reason not to do so.  But one of the things I have always liked about
git is that no matter how I prepare content, the output is always the
same.

But maybe this is just me being a curmudgeonly old-timer. Feel free to
ignore.

-Peff

On Tue, 19 Aug 2008, Jeff King wrote:

On Mon, Aug 18, 2008 at 08:22:19PM -0700, Junio C Hamano wrote:

quoted

I do not know if I like the end result, but here is a patch to make the
traditional a/ and b/ prefix more mnemonic.

Hmm. Something deep in my gut doesn't like this, just because I like the
fact that no matter how I prepare a diff (and I do tend to do it
different ways and post to the mailing list) it always ends up the same.
For example, I sometimes "hand-generate" patch messages meant to be
applied by git-am by doing a diff between the working tree and index and
pasting the result into an email. It just feels a bit wrong for it not
to be the exact output I would get from commiting and running
format-patch.

Hmm... everybody who doesn't like it is concerned about scripts and 
sending it places, while the people who like it seem to be interested in 
looking at the output. Maybe there should be an option that controls it, 
with the default being to use -a+b for pipelines and informational stuff 
for pager?

It seems to me like, in output for user consumption, the information is 
useful, while in output for non-user consumption, the information is 
overly personal. But that's easy enough to detect...

(For that matter, maybe format-patch should be able to handle uncommitted 
changes, and should hide what it did? What's with all these people faking 
format-patch output with other commands, rather than having format-patch 
actually generate suitable output in their situations?)

	-Daniel
*This .sig left intentionally blank*

On Tue, Aug 19, 2008 at 02:39:22PM -0400, Daniel Barkalow wrote:

Hmm... everybody who doesn't like it is concerned about scripts and 
sending it places, while the people who like it seem to be interested in 
looking at the output. Maybe there should be an option that controls it, 
with the default being to use -a+b for pipelines and informational stuff 
for pager?

To clarify my statement: no, I'm concerned about looking at it. That is,
I don't think it will break scripts, but I think the output is
potentially confusing to humans.

But like I said before, it's just my intuition; I don't have real facts
to back it up, so feel free to ignore.

(For that matter, maybe format-patch should be able to handle uncommitted 
changes, and should hide what it did? What's with all these people faking 
format-patch output with other commands, rather than having format-patch 
actually generate suitable output in their situations?)

I do it because I haven't actually committed the content.  I dump the
diff right into an email I'm already writing.

-Peff

On Tue, 19 Aug 2008, Jeff King wrote:

On Tue, Aug 19, 2008 at 02:39:22PM -0400, Daniel Barkalow wrote:

quoted

Hmm... everybody who doesn't like it is concerned about scripts and 
sending it places, while the people who like it seem to be interested in 
looking at the output. Maybe there should be an option that controls it, 
with the default being to use -a+b for pipelines and informational stuff 
for pager?

To clarify my statement: no, I'm concerned about looking at it. That is,
I don't think it will break scripts, but I think the output is
potentially confusing to humans.

Humans being recipients of emails, or humans being the users who typed the 
command? Unless you're cut-and-pasting out of a pager (which never works 
well for me if it's long enough to include diff headers, context, and some 
change), recipients of emails would get what scripts get. (I personnaly do 
that as "git diff > temp.patch" and read temp.patch into my mailer; this 
doesn't trigger starting a pager, and wouldn't trigger the default to be 
informative prefixes.)

But like I said before, it's just my intuition; I don't have real facts
to back it up, so feel free to ignore.

quoted

(For that matter, maybe format-patch should be able to handle uncommitted 
changes, and should hide what it did? What's with all these people faking 
format-patch output with other commands, rather than having format-patch 
actually generate suitable output in their situations?)

I do it because I haven't actually committed the content.  I dump the
diff right into an email I'm already writing.

Yeah, that's why I think that format-patch should work on content that you 
haven't committed, generating something you can dump right into an email 
(with the --- and diffstat that you'd get if you actually did commit and 
use format-patch now).

	-Daniel
*This .sig left intentionally blank*

On Tue, Aug 19, 2008 at 02:57:04PM -0400, Daniel Barkalow wrote:

Humans being recipients of emails, or humans being the users who typed the 
command? Unless you're cut-and-pasting out of a pager (which never works 

I meant the recipients of the emails.

well for me if it's long enough to include diff headers, context, and some 
change), recipients of emails would get what scripts get. (I personnaly do 
that as "git diff > temp.patch" and read temp.patch into my mailer; this 
doesn't trigger starting a pager, and wouldn't trigger the default to be 
informative prefixes.)

OK, I didn't read your mail carefully enough. Yes, I do the same thing,
so the "do this only if pager" rule would meet my requirement. OTOH, I
don't know if that would satisfy the people who want this feature (but I
will let them speak for themselves).

Yeah, that's why I think that format-patch should work on content that you 
haven't committed, generating something you can dump right into an email 
(with the --- and diffstat that you'd get if you actually did commit and 
use format-patch now).

It's not clear to me:

  - how you would tell format-patch that's what you wanted to dump

  - what parts would be included. There's no commit message or author.
    We could guess at the author as if you were about to commit this.

  - how this would be any real improvement over "git diff --stat -p". In
    fact, I like the fact that I get _just_ the diff, which I then
    paste. The headers would just be clutter I would have to delete.

-Peff

On Tue, 19 Aug 2008, Jeff King wrote:

On Tue, Aug 19, 2008 at 02:57:04PM -0400, Daniel Barkalow wrote:

quoted

Humans being recipients of emails, or humans being the users who typed the 
command? Unless you're cut-and-pasting out of a pager (which never works 

I meant the recipients of the emails.

quoted

well for me if it's long enough to include diff headers, context, and some 
change), recipients of emails would get what scripts get. (I personnaly do 
that as "git diff > temp.patch" and read temp.patch into my mailer; this 
doesn't trigger starting a pager, and wouldn't trigger the default to be 
informative prefixes.)

OK, I didn't read your mail carefully enough. Yes, I do the same thing,
so the "do this only if pager" rule would meet my requirement. OTOH, I
don't know if that would satisfy the people who want this feature (but I
will let them speak for themselves).

Ah, okay. I feel like the main application for this is "I typed some git 
diff command, started looking at it, my phone rang, I took the call, and 
now I don't know what I'm looking at, and the pager hides the command 
line, but quitting the pager loses my place." At least, that's the 
situation I'm often in.

quoted

Yeah, that's why I think that format-patch should work on content that you 
haven't committed, generating something you can dump right into an email 
(with the --- and diffstat that you'd get if you actually did commit and 
use format-patch now).

It's not clear to me:

  - how you would tell format-patch that's what you wanted to dump

Maybe an option? Maybe it should include it if the working tree is dirty?

  - what parts would be included. There's no commit message or author.
    We could guess at the author as if you were about to commit this.

Probably it should start just after the message, since that's what you've 
presumably got elsewhere.

  - how this would be any real improvement over "git diff --stat -p". In
    fact, I like the fact that I get _just_ the diff, which I then
    paste. The headers would just be clutter I would have to delete.

That all-important "---" line? But I think the real advantage is that 
people who don't know that "git diff --stat -p" is the standard info for a 
patch email would be able to run the same command as usual.

	-Daniel
*This .sig left intentionally blank*

On Tue, Aug 19, 2008 at 03:42:01PM -0400, Daniel Barkalow wrote:

On Tue, 19 Aug 2008, Jeff King wrote:
Ah, okay. I feel like the main application for this is "I typed some git 
diff command, started looking at it, my phone rang, I took the call, and 
now I don't know what I'm looking at, and the pager hides the command 
line, but quitting the pager loses my place." At least, that's the 
situation I'm often in.

Press ctrl-z. ;-)

quoted

quoted

Yeah, that's why I think that format-patch should work on content that you 
haven't committed, generating something you can dump right into an email 
(with the --- and diffstat that you'd get if you actually did commit and 
use format-patch now).

Hmm, and why don't you actually do the commit after all? You can compose
all the details within the commit and you can do the commit on a
separate branch or git reset HEAD^ afterwards if you don't want to keep
it around.

-- 
				Petr "Pasky" Baudis
The next generation of interesting software will be done
on the Macintosh, not the IBM PC.  -- Bill Gates

On Tue, 19 Aug 2008, Petr Baudis wrote:

On Tue, Aug 19, 2008 at 03:42:01PM -0400, Daniel Barkalow wrote:

quoted

On Tue, 19 Aug 2008, Jeff King wrote:
Ah, okay. I feel like the main application for this is "I typed some git 
diff command, started looking at it, my phone rang, I took the call, and 
now I don't know what I'm looking at, and the pager hides the command 
line, but quitting the pager loses my place." At least, that's the 
situation I'm often in.

Press ctrl-z. ;-)

quoted

quoted

quoted

Yeah, that's why I think that format-patch should work on content that you 
haven't committed, generating something you can dump right into an email 
(with the --- and diffstat that you'd get if you actually did commit and 
use format-patch now).

Hmm, and why don't you actually do the commit after all? You can compose
all the details within the commit and you can do the commit on a
separate branch or git reset HEAD^ afterwards if you don't want to keep
it around.

I personally almost always do something like:

$ git checkout -b informational-diff-prefixes
$ git commit -a
$ git show HEAD > temp.patch
$ git checkout master

But then I tend to use "checkout -b; commit -a; checkout" instead of 
"reset --hard" to get rid of unwanted local changes anyway these days.

	-Daniel
*This .sig left intentionally blank*