[PATCH] git log [diff-tree options]...

[PATCH] git log [diff-tree options]...

[Junio C Hamano]

And this makes "git log" to take common diff-tree options, so
that it can be used as "git whatchanged".

The recent revision walker updates by Linus to make path
limiting low-latency helps this quite a bit.

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

---

 git.c |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

52b70d56bd23811003a72866cc23a0a44b9da1b7
diff --git a/git.c b/git.c
index fa58232..8776088 100644
--- a/git.c
+++ b/git.c
@@ -16,6 +16,8 @@ #include "common-cmds.h"
 #include "cache.h"
 #include "commit.h"
 #include "revision.h"
+#include "diff.h"
+#include "log-tree.h"
 
 #ifndef PATH_MAX
 # define PATH_MAX 4096
@@ -285,7 +287,10 @@ static int cmd_log(int argc, const char 
 	int abbrev = DEFAULT_ABBREV;
 	int abbrev_commit = 0;
 	const char *commit_prefix = "commit ";
+	struct log_tree_opt opt;
+	int do_diff = 0;
 
+	init_log_tree_opt(&opt);
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
 	while (1 < argc) {
 		const char *arg = argv[1];
@@ -310,9 +315,31 @@ static int cmd_log(int argc, const char 
 			else if (40 < abbrev)
 				abbrev = 40;
 		}
-		else
+		else {
+			int cnt = log_tree_opt_parse(&opt, argv+1, argc-1);
+			if (0 < cnt) {
+				do_diff = 1;
+				argv += cnt;
+				argc -= cnt;
+				continue;
+			}
 			die("unrecognized argument: %s", arg);
+		}
+
 		argc--; argv++;
+	}
+	if (do_diff) {
+		opt.diffopt.abbrev = abbrev;
+		opt.verbose_header = 0;
+		opt.always_show_header = 0;
+		opt.no_commit_id = 1;
+		if (opt.combine_merges)
+			opt.ignore_merges = 0;
+		if (opt.dense_combined_merges)
+			opt.diffopt.output_format = DIFF_FORMAT_PATCH;
+		if (opt.diffopt.output_format == DIFF_FORMAT_PATCH)
+			opt.diffopt.recursive = 1;
+		diff_setup_done(&opt.diffopt);
 	}
 
 	prepare_revision_walk(&rev);
@@ -350,6 +377,9 @@ static int cmd_log(int argc, const char 
 		pretty_print_commit(commit_format, commit, ~0, buf,
 				    LOGSIZE, abbrev);
 		printf("%s\n", buf);
+
+		if (do_diff)
+			log_tree_commit(&opt, commit);
 	}
 	free(buf);
 	return 0;
-- 
1.2.6.gad0b

Re: [PATCH] git log [diff-tree options]...

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> And this makes "git log" to take common diff-tree options, so
> that it can be used as "git whatchanged".

BTW, this is *not* "whatchanged" in that it does not omit the
log when there is no diff output.  It may not matter much --
when we give paths limiter to the command, the commits that are
shown are already limited by those paths limiter.

Re: [PATCH] git log [diff-tree options]...

[Linus Torvalds]

On Sun, 9 Apr 2006, Junio C Hamano wrote:
>
> And this makes "git log" to take common diff-tree options, so
> that it can be used as "git whatchanged".

I wonder... This all looks fine, but there are actually two different 
"diffs" that can be shown for "git log --diff <pathlimiter>":

 - the whole diff for a commit

 - the path-limited diff

and I think we'd likely want to have some way to select the output. 
Probably with the path-limited diff being the default (that's what 
"git-whatchanged" does), but perhaps with "--full-diff" showing the whole 
commit diff (which is what "gitk" does).

		Linus

Re: [PATCH] git log [diff-tree options]...

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> I wonder... This all looks fine, but there are actually two different 
> "diffs" that can be shown for "git log --diff <pathlimiter>":
>
>  - the whole diff for a commit
>  - the path-limited diff

Yes, exactly the same way sometimes you would want just pickaxe,
sometimes you would want it with --pickaxe-all.

Also, I might have to rethink --max-count logic -- I think it is
reasonable to skip the commit when doing limiting by diff like
"whatchanged" does, but one thing I find suboptimal with the
current whatchanged is that it does not count commits that are
actually shown (it counts what the upstream rev-list feeds
diff-tree).  With the "git log --diff" based whatchanged, it
becomes trivial to skip the revs->max_count limiting and have
the caller count the commits it actually does something
user-visible to, instead of counting the commits it pulled out
of get_revision().

BTW I think I could remove the log message generation part of
"git log" and have it use the one in log-tree (which I will
probably rewrite not to format the message into the static
this_header[] buffer when it is not shown).

Another thing that might be useful is to teach diff-* to do the
diffstat part internally.  After that is in place we could
introduce --pretty=patch to have "git log" produce format-patch
compatible output.

Re: [PATCH] git log [diff-tree options]...

[Linus Torvalds]

On Sun, 9 Apr 2006, Junio C Hamano wrote:
> 
> Also, I might have to rethink --max-count logic -- I think it is
> reasonable to skip the commit when doing limiting by diff like
> "whatchanged" does, but one thing I find suboptimal with the
> current whatchanged is that it does not count commits that are
> actually shown (it counts what the upstream rev-list feeds
> diff-tree).  With the "git log --diff" based whatchanged, it
> becomes trivial to skip the revs->max_count limiting and have
> the caller count the commits it actually does something
> user-visible to, instead of counting the commits it pulled out
> of get_revision().

Well, on the other hand, the new "git log --diff" should get the revision 
counting right even if it's _not_ done by the caller.

Really, the only reason "git-whatchanged" exists at all is that it used to 
be originally impossible, and later on too expensive to do the commit- 
limiting by pathname. With the new incremental path-limiting, the reason 
for "git-whatchanged" simply goes away.

So I'd suggest:
 - drop git-whatchanged entirely
 - keep it - for historical reasons - as a internal shorthand, and just 
   turn it into "git log --diff -cc"

and everybody will be happy (yeah, it will show a few merge commits 
without diffs, because the diffs end up being uninteresting, but that's 
_fine_, even if it's not 100% the same thing git-whatchanged used to do)

			Linus

Re: [PATCH] git log [diff-tree options]...

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Well, on the other hand, the new "git log --diff" should get the revision 
> counting right even if it's _not_ done by the caller.

Not if the user uses --diff-filter and/or --pickaxe, and after
we start omitting the log message part when no diff is output.

> So I'd suggest:
>  - drop git-whatchanged entirely
>  - keep it - for historical reasons - as a internal shorthand, and just 
>    turn it into "git log --diff -cc"
>
> and everybody will be happy (yeah, it will show a few merge commits 
> without diffs, because the diffs end up being uninteresting, but that's 
> _fine_, even if it's not 100% the same thing git-whatchanged used to do)

I tend to agree.  A merge commit touching a path but not
actually changing the contents of the path from parents might be
a significant event.

Re: [PATCH] git log [diff-tree options]...

[Linus Torvalds]

On Sun, 9 Apr 2006, Junio C Hamano wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > Well, on the other hand, the new "git log --diff" should get the revision 
> > counting right even if it's _not_ done by the caller.
> 
> Not if the user uses --diff-filter and/or --pickaxe, and after
> we start omitting the log message part when no diff is output.

Fair enough. At that point the counting does have to be done in the 
caller, I guess.

> A merge commit touching a path but not actually changing the contents of 
> the path from parents might be a significant event.

Yes. The fact that git-whatchanged happens to ignore such things right now 
is just a implementation detail, not a "good thing". The new git log seems 
to be better in pretty much all respects.

The bigger conceptual difference is actually that once you do revision 
pruning based on the pathname limiter, we prune away parents of merges 
that seem "uninteresting". So before, when you had the same change come 
through two different branches, "git-whatchanged" would actually show it 
twice, while the new "git log" approach would tend to show it just once 
(because it would pick one of the histories and ignore the other).

I think that's fine (and probably even preferable), but it's another 
example of something where we might want to have an option to not 
simplify the merge history. It's likely that nobody will ever care, but 
who knows..

			Linus

Re: [PATCH] git log [diff-tree options]...

[Johannes Schindelin]

Hi,

On Sun, 9 Apr 2006, Linus Torvalds wrote:

>  - keep it - for historical reasons - as a internal shorthand, and just 
>    turn it into "git log --diff -cc"

It is "git log --cc", right? And BTW, I was burnt by the difference of 
"git-log" and "git log" this time. "git-log" does not understand "--cc". 
Could we kill "git-log", please?

Ciao,
Dscho

Re: [PATCH] git log [diff-tree options]...

[Johannes Schindelin]

Hi,

On Sun, 9 Apr 2006, Johannes Schindelin wrote:

> On Sun, 9 Apr 2006, Linus Torvalds wrote:
> 
> >  - keep it - for historical reasons - as a internal shorthand, and just 
> >    turn it into "git log --diff -cc"
> 
> It is "git log --cc", right?

Like this?

---

 git.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

751e205a9ffd3a55094a0c0f657735023776cf74
diff --git a/git.c b/git.c
index 8776088..3a94afa 100644
--- a/git.c
+++ b/git.c
@@ -385,6 +385,13 @@ static int cmd_log(int argc, const char 
 	return 0;
 }
 
+static int cmd_whatchanged(int argc, const char **argv, char **envp)
+{
+	memmove(argv + 2, argv + 1, argc - 1);
+	argv[1] = "--cc";
+	return cmd_log(argc + 1, argv, envp);
+}
+
 static void handle_internal_command(int argc, const char **argv, char **envp)
 {
 	const char *cmd = argv[0];
@@ -395,6 +402,7 @@ static void handle_internal_command(int 
 		{ "version", cmd_version },
 		{ "help", cmd_help },
 		{ "log", cmd_log },
+		{ "whatchanged", cmd_whatchanged },
 	};
 	int i;
 
-- 
1.2.0.g61002-dirty

Re: [PATCH] git log [diff-tree options]...

[Timo Hirvonen]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> +static int cmd_whatchanged(int argc, const char **argv, char **envp)
> +{
> +	memmove(argv + 2, argv + 1, argc - 1);

Shouldn't the size be sizeof(char *) * argc (NULL terminated array)?
There's also overflow...

-- 
http://onion.dynserv.net/~timo/

Re: [PATCH] git log [diff-tree options]...

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Sun, 9 Apr 2006, Johannes Schindelin wrote:
>
>> On Sun, 9 Apr 2006, Linus Torvalds wrote:
>> 
>> >  - keep it - for historical reasons - as a internal shorthand, and just 
>> >    turn it into "git log --diff -cc"
>> 
>> It is "git log --cc", right?
>
> Like this?

I do not think so.  You should default to --cc only there is no
explicit command line stuff from the user.

Re: [PATCH] git log [diff-tree options]...

[Linus Torvalds]

On Sun, 9 Apr 2006, Junio C Hamano wrote:
> 
> I do not think so.  You should default to --cc only there is no
> explicit command line stuff from the user.

Actually, even that would be wrong, when I think more about it. The 
default for "git-whatchanged" is to do diffing, but default to the "raw" 
diff (just "-r" for recursive).

So the most appropriate default set of flags is likely "-r -c", which also 
means that any subsequent explicit command line stuff will override it (ie 
adding a "-p" should automatically do the right thing).

But the "memmove()" to move the arguments around was definitely broken. 
Much better to just initialize the diff flags manually, I think.

		Linus

Re: [PATCH] git log [diff-tree options]...

[Johannes Schindelin]

Hi,

On Mon, 10 Apr 2006, Timo Hirvonen wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > +static int cmd_whatchanged(int argc, const char **argv, char **envp)
> > +{
> > +	memmove(argv + 2, argv + 1, argc - 1);
> 
> Shouldn't the size be sizeof(char *) * argc (NULL terminated array)?
> There's also overflow...

Yeah, this was some useless late-night coding. But you get the idea.

Ciao,
Dscho

Re: [PATCH] git log [diff-tree options]...

[Johannes Schindelin]

Hi,

On Sun, 9 Apr 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Hi,
> >
> > On Sun, 9 Apr 2006, Johannes Schindelin wrote:
> >
> >> On Sun, 9 Apr 2006, Linus Torvalds wrote:
> >> 
> >> >  - keep it - for historical reasons - as a internal shorthand, and just 
> >> >    turn it into "git log --diff -cc"
> >> 
> >> It is "git log --cc", right?
> >
> > Like this?
> 
> I do not think so.  You should default to --cc only there is no
> explicit command line stuff from the user.

Well, my idea was to get rid of the whatchanged script, and deprecate the 
internal whatchanged. IMHO "git log" is much faster typed than "git 
whatchanged", especially if you have no completion installed. I, for one, 
will never ever again use whatchanged.

But you and the list gave me bad marks for that patch, and rightfully so.

Ciao,
Dscho

Re: [PATCH] git log [diff-tree options]...

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 9 Apr 2006, Junio C Hamano wrote:
>>
>> And this makes "git log" to take common diff-tree options, so
>> that it can be used as "git whatchanged".
>
> I wonder... This all looks fine, but there are actually two different 
> "diffs" that can be shown for "git log --diff <pathlimiter>":
>
>  - the whole diff for a commit
>
>  - the path-limited diff
>
> and I think we'd likely want to have some way to select the output. 
> Probably with the path-limited diff being the default (that's what 
> "git-whatchanged" does), but perhaps with "--full-diff" showing the whole 
> commit diff (which is what "gitk" does).

Yes, but it turns out it is a bit tricky, given that the way
tree-diff.c is written it pretty much assumes the same pathspec
is used for diff operation during the lifetime of the process.

I think we should move (nr_paths, pathlens and paths) to struct
diff_options.  Two patches to follow shortly.

[PATCH] tree-diff: do not assume we use only one pathspec

[Junio C Hamano]

The way tree-diff was set up assumed we would use only one set
of pathspec during the entire life of the program.  Move the
pathspec related static variables out to diff_options structure
so that we can filter commits with one set of paths while show
the actual diffs using different set of paths.

I suspect this breaks blame.c, and makes "git log paths..." to
default to the --full-diff, the latter of which is dealt with
the next commit.

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

---

 diff-tree.c |    2 +-
 diff.h      |    6 +++++-
 revision.c  |    2 +-
 tree-diff.c |   46 +++++++++++++++++++++++++---------------------
 4 files changed, 32 insertions(+), 24 deletions(-)

be7b412689f723a1008b40712d9f0b96773ad81a
diff --git a/diff-tree.c b/diff-tree.c
index 2a088d1..2b79dd0 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -120,7 +120,7 @@ int main(int argc, const char **argv)
 	if (opt->diffopt.output_format == DIFF_FORMAT_PATCH)
 		opt->diffopt.recursive = 1;
 
-	diff_tree_setup_paths(get_pathspec(prefix, argv));
+	diff_tree_setup_paths(get_pathspec(prefix, argv), opt);
 	diff_setup_done(&opt->diffopt);
 
 	switch (nr_sha1) {
diff --git a/diff.h b/diff.h
index a02ef28..cc7cc62 100644
--- a/diff.h
+++ b/diff.h
@@ -38,11 +38,15 @@ struct diff_options {
 	int setup;
 	int abbrev;
 
+	int nr_paths;
+	const char **paths;
+	int *pathlens;
 	change_fn_t change;
 	add_remove_fn_t add_remove;
 };
 
-extern void diff_tree_setup_paths(const char **paths);
+extern void diff_tree_setup_paths(const char **paths, struct diff_options *);
+extern void diff_tree_release_paths(struct diff_options *);
 extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 		     const char *base, struct diff_options *opt);
 extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
diff --git a/revision.c b/revision.c
index fe26562..634f9a5 100644
--- a/revision.c
+++ b/revision.c
@@ -707,7 +707,7 @@ int setup_revisions(int argc, const char
 		revs->limited = 1;
 
 	if (revs->prune_data) {
-		diff_tree_setup_paths(revs->prune_data);
+		diff_tree_setup_paths(revs->prune_data, &diff_opt);
 		revs->prune_fn = try_to_simplify_commit;
 	}
 
diff --git a/tree-diff.c b/tree-diff.c
index 701fbba..1cdf8aa 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -5,11 +5,6 @@ #include "cache.h"
 #include "diff.h"
 #include "tree.h"
 
-// What paths are we interested in?
-static int nr_paths = 0;
-static const char **paths = NULL;
-static int *pathlens = NULL;
-
 static char *malloc_base(const char *base, const char *path, int pathlen)
 {
 	int baselen = strlen(base);
@@ -72,14 +67,14 @@ static int compare_tree_entry(struct tre
 	return 0;
 }
 
-static int interesting(struct tree_desc *desc, const char *base)
+static int interesting(struct tree_desc *desc, const char *base, struct diff_options *opt)
 {
 	const char *path;
 	unsigned mode;
 	int i;
 	int baselen, pathlen;
 
-	if (!nr_paths)
+	if (!opt->nr_paths)
 		return 1;
 
 	(void)tree_entry_extract(desc, &path, &mode);
@@ -87,9 +82,9 @@ static int interesting(struct tree_desc 
 	pathlen = strlen(path);
 	baselen = strlen(base);
 
-	for (i=0; i < nr_paths; i++) {
-		const char *match = paths[i];
-		int matchlen = pathlens[i];
+	for (i=0; i < opt->nr_paths; i++) {
+		const char *match = opt->paths[i];
+		int matchlen = opt->pathlens[i];
 
 		if (baselen >= matchlen) {
 			/* If it doesn't match, move along... */
@@ -129,7 +124,7 @@ static int interesting(struct tree_desc 
 static void show_tree(struct diff_options *opt, const char *prefix, struct tree_desc *desc, const char *base)
 {
 	while (desc->size) {
-		if (interesting(desc, base))
+		if (interesting(desc, base, opt))
 			show_entry(opt, prefix, desc, base);
 		update_tree_entry(desc);
 	}
@@ -167,11 +162,11 @@ static int show_entry(struct diff_option
 int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
 {
 	while (t1->size | t2->size) {
-		if (nr_paths && t1->size && !interesting(t1, base)) {
+		if (opt->nr_paths && t1->size && !interesting(t1, base, opt)) {
 			update_tree_entry(t1);
 			continue;
 		}
-		if (nr_paths && t2->size && !interesting(t2, base)) {
+		if (opt->nr_paths && t2->size && !interesting(t2, base, opt)) {
 			update_tree_entry(t2);
 			continue;
 		}
@@ -229,19 +224,28 @@ static int count_paths(const char **path
 	return i;
 }
 
-void diff_tree_setup_paths(const char **p)
+void diff_tree_release_paths(struct diff_options *opt)
 {
+	free(opt->pathlens);
+}
+
+void diff_tree_setup_paths(const char **p, struct diff_options *opt)
+{
+	opt->nr_paths = 0;
+	opt->pathlens = NULL;
+	opt->paths = NULL;
+
 	if (p) {
 		int i;
 
-		paths = p;
-		nr_paths = count_paths(paths);
-		if (nr_paths == 0) {
-			pathlens = NULL;
+		opt->paths = p;
+		opt->nr_paths = count_paths(p);
+		if (opt->nr_paths == 0) {
+			opt->pathlens = NULL;
 			return;
 		}
-		pathlens = xmalloc(nr_paths * sizeof(int));
-		for (i=0; i<nr_paths; i++)
-			pathlens[i] = strlen(paths[i]);
+		opt->pathlens = xmalloc(opt->nr_paths * sizeof(int));
+		for (i=0; i < opt->nr_paths; i++)
+			opt->pathlens[i] = strlen(p[i]);
 	}
 }
-- 
1.3.0.rc3.g910a

[PATCH] git log --full-diff

[Junio C Hamano]

Without this flag, "git log -p paths..." shows commits that
touch the specified paths, and diffs about the same specified
paths.  With this, the full diff is shown for commits that touch
the specified paths.

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

---

 git.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

fd3ed3d9667e034b90aad02b4f9e5efcc61f1ce3
diff --git a/git.c b/git.c
index 8776088..ad896da 100644
--- a/git.c
+++ b/git.c
@@ -288,7 +288,9 @@ static int cmd_log(int argc, const char 
 	int abbrev_commit = 0;
 	const char *commit_prefix = "commit ";
 	struct log_tree_opt opt;
+	int shown = 0;
 	int do_diff = 0;
+	int full_diff = 0;
 
 	init_log_tree_opt(&opt);
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
@@ -315,6 +317,10 @@ static int cmd_log(int argc, const char 
 			else if (40 < abbrev)
 				abbrev = 40;
 		}
+		else if (!strcmp(arg, "--full-diff")) {
+			do_diff = 1;
+			full_diff = 1;
+		}
 		else {
 			int cnt = log_tree_opt_parse(&opt, argv+1, argc-1);
 			if (0 < cnt) {
@@ -328,6 +334,7 @@ static int cmd_log(int argc, const char 
 
 		argc--; argv++;
 	}
+
 	if (do_diff) {
 		opt.diffopt.abbrev = abbrev;
 		opt.verbose_header = 0;
@@ -339,12 +346,16 @@ static int cmd_log(int argc, const char 
 			opt.diffopt.output_format = DIFF_FORMAT_PATCH;
 		if (opt.diffopt.output_format == DIFF_FORMAT_PATCH)
 			opt.diffopt.recursive = 1;
+		if (!full_diff && rev.prune_data)
+			diff_tree_setup_paths(rev.prune_data, &opt.diffopt);
 		diff_setup_done(&opt.diffopt);
 	}
 
 	prepare_revision_walk(&rev);
 	setup_pager();
 	while ((commit = get_revision(&rev)) != NULL) {
+		if (commit_format != CMIT_FMT_ONELINE && shown)
+			putchar('\n');
 		fputs(commit_prefix, stdout);
 		if (abbrev_commit && abbrev)
 			fputs(find_unique_abbrev(commit->object.sha1, abbrev),
@@ -377,9 +388,9 @@ static int cmd_log(int argc, const char 
 		pretty_print_commit(commit_format, commit, ~0, buf,
 				    LOGSIZE, abbrev);
 		printf("%s\n", buf);
-
 		if (do_diff)
 			log_tree_commit(&opt, commit);
+		shown = 1;
 	}
 	free(buf);
 	return 0;
-- 
1.3.0.rc3.g910a

Re: [PATCH] git log [diff-tree options]...

[Linus Torvalds]

On Mon, 10 Apr 2006, Junio C Hamano wrote:
> >
> > and I think we'd likely want to have some way to select the output. 
> > Probably with the path-limited diff being the default (that's what 
> > "git-whatchanged" does), but perhaps with "--full-diff" showing the whole 
> > commit diff (which is what "gitk" does).
> 
> Yes, but it turns out it is a bit tricky, given that the way
> tree-diff.c is written it pretty much assumes the same pathspec
> is used for diff operation during the lifetime of the process.

Hmm.. The hacky approach would seem to be to just call 
"diff_tree_setup_paths()" just before doing the diff (and re-set it 
afterwards). That should work, I don't think we keep any real state 
around.

> I think we should move (nr_paths, pathlens and paths) to struct
> diff_options.  Two patches to follow shortly.

That certainly seems to be the cleanest option. Not just because it should 
solve this problem, but because it's also what looks most natural anyway 
(and matches the "struct revs" approach).

		Linus

Re: [PATCH] git log [diff-tree options]...

[Randal L. Schwartz]

>>>>> "Johannes" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Johannes> Hi,
Johannes> On Sun, 9 Apr 2006, Linus Torvalds wrote:

>> - keep it - for historical reasons - as a internal shorthand, and just 
>> turn it into "git log --diff -cc"

Johannes> It is "git log --cc", right? And BTW, I was burnt by the difference of 
Johannes> "git-log" and "git log" this time. "git-log" does not understand "--cc". 
Johannes> Could we kill "git-log", please?

Wait.  Why is there a git log and a git-log?  Shouldn't those
be *absolutely* identical?  Or have we now finally diverged, violating
rules that were established earlier?

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: [PATCH] git log [diff-tree options]...

[Junio C Hamano]

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> Wait.  Why is there a git log and a git-log?  Shouldn't those
> be *absolutely* identical?  Or have we now finally diverged, violating
> rules that were established earlier?

What rule ;-)?

I kept "git-log.sh" for two reasons.  (1) to see how long it
takes people to notice, and (2) to have a handy way to verify
potential regressions.

There is no reason for them to be *absolutely* identical -- if
the git.c embedded one turns out to be usable, useful and even
superiour, git-log.sh based one should be retired, and will be
made again as synonyms, perhaps like this:

--
diff --git a/Makefile b/Makefile
index a979205..5239526 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,7 @@ SCRIPT_SH = \
 	git-add.sh git-bisect.sh git-branch.sh git-checkout.sh \
 	git-cherry.sh git-clean.sh git-clone.sh git-commit.sh \
 	git-count-objects.sh git-diff.sh git-fetch.sh \
-	git-format-patch.sh git-log.sh git-ls-remote.sh \
+	git-format-patch.sh git-ls-remote.sh \
 	git-merge-one-file.sh git-parse-remote.sh \
 	git-prune.sh git-pull.sh git-push.sh git-rebase.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
@@ -167,6 +167,8 @@ PROGRAMS = \
 	git-name-rev$X git-pack-redundant$X git-repo-config$X git-var$X \
 	git-describe$X git-merge-tree$X git-blame$X git-imap-send$X
 
+BUILT_INS = git-log$X
+
 # what 'all' will build and 'install' will install, in gitexecdir
 ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS)
 
@@ -448,7 +450,7 @@ LIB_OBJS += $(COMPAT_OBJS)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 ### Build rules
 
-all: $(ALL_PROGRAMS) git$X gitk
+all: $(ALL_PROGRAMS) git$X $(BUILT_INS) gitk
 
 all:
 	$(MAKE) -C templates
@@ -460,6 +462,9 @@ git$X: git.c common-cmds.h $(GITLIBS)
 	$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
 		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) \
 		$(ALL_LDFLAGS) $(LIBS)
+
+$(BUILT_INS): git$X
+	rm -f $@ && ln git$X $@
 
 common-cmds.h: Documentation/git-*.txt
 	./generate-cmdlist.sh > $@
@@ -642,7 +647,7 @@ ### Cleaning rules
 
 clean:
 	rm -f *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o xdiff/*.o \
-		$(LIB_FILE) $(XDIFF_LIB)
+		$(LIB_FILE) $(XDIFF_LIB) $(BUILT_INS)
 	rm -f $(ALL_PROGRAMS) git$X
 	rm -f *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags
 	rm -rf $(GIT_TARNAME)

Re: [PATCH] git log [diff-tree options]...

[Petr Baudis]

Dear diary, on Tue, Apr 11, 2006 at 02:12:11AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> There is no reason for them to be *absolutely* identical

One is that people expect them to be. It's been implied that "git xyzzy"
is functionally fully equivalent to "git-xyzzy" for too long to silently
break this now.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

Re: [PATCH] git log [diff-tree options]...

[Randal L. Schwartz]

>>>>> "Petr" == Petr Baudis <pasky@suse.cz> writes:

Petr> Dear diary, on Tue, Apr 11, 2006 at 02:12:11AM CEST, I got a letter
Petr> where Junio C Hamano <junkio@cox.net> said that...
>> There is no reason for them to be *absolutely* identical

Petr> One is that people expect them to be. It's been implied that "git xyzzy"
Petr> is functionally fully equivalent to "git-xyzzy" for too long to silently
Petr> break this now.

Yes, this is what I was referencing.  I would *never* have considered
that git-log and "git log" be different things now that it's been firmly
engrained that they are the same.  Please don't violate the model.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!