* [PATCH] git log [diff-tree options]... @ 2006-04-09 9:04 Junio C Hamano 2006-04-09 9:16 ` Junio C Hamano 2006-04-09 16:53 ` Linus Torvalds 0 siblings, 2 replies; 22+ messages in thread From: Junio C Hamano @ 2006-04-09 9:04 UTC (permalink / raw) To: Linus Torvalds; +Cc: git 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 9:04 [PATCH] git log [diff-tree options] Junio C Hamano @ 2006-04-09 9:16 ` Junio C Hamano 2006-04-09 16:53 ` Linus Torvalds 1 sibling, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2006-04-09 9:16 UTC (permalink / raw) To: git; +Cc: Linus Torvalds 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 9:04 [PATCH] git log [diff-tree options] Junio C Hamano 2006-04-09 9:16 ` Junio C Hamano @ 2006-04-09 16:53 ` Linus Torvalds 2006-04-09 18:46 ` Junio C Hamano 2006-04-10 23:42 ` Junio C Hamano 1 sibling, 2 replies; 22+ messages in thread From: Linus Torvalds @ 2006-04-09 16:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 16:53 ` Linus Torvalds @ 2006-04-09 18:46 ` Junio C Hamano 2006-04-09 19:02 ` Linus Torvalds 2006-04-10 23:42 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2006-04-09 18:46 UTC (permalink / raw) To: Linus Torvalds; +Cc: git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 18:46 ` Junio C Hamano @ 2006-04-09 19:02 ` Linus Torvalds 2006-04-09 19:08 ` Junio C Hamano 2006-04-09 21:13 ` Johannes Schindelin 0 siblings, 2 replies; 22+ messages in thread From: Linus Torvalds @ 2006-04-09 19:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 19:02 ` Linus Torvalds @ 2006-04-09 19:08 ` Junio C Hamano 2006-04-09 19:26 ` Linus Torvalds 2006-04-09 21:13 ` Johannes Schindelin 1 sibling, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2006-04-09 19:08 UTC (permalink / raw) To: Linus Torvalds; +Cc: git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 19:08 ` Junio C Hamano @ 2006-04-09 19:26 ` Linus Torvalds 0 siblings, 0 replies; 22+ messages in thread From: Linus Torvalds @ 2006-04-09 19:26 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 19:02 ` Linus Torvalds 2006-04-09 19:08 ` Junio C Hamano @ 2006-04-09 21:13 ` Johannes Schindelin 2006-04-09 22:01 ` Johannes Schindelin 2006-04-11 0:04 ` Randal L. Schwartz 1 sibling, 2 replies; 22+ messages in thread From: Johannes Schindelin @ 2006-04-09 21:13 UTC (permalink / raw) To: Linus Torvalds; +Cc: Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 21:13 ` Johannes Schindelin @ 2006-04-09 22:01 ` Johannes Schindelin 2006-04-09 22:22 ` Timo Hirvonen 2006-04-09 23:51 ` Junio C Hamano 2006-04-11 0:04 ` Randal L. Schwartz 1 sibling, 2 replies; 22+ messages in thread From: Johannes Schindelin @ 2006-04-09 22:01 UTC (permalink / raw) To: Linus Torvalds; +Cc: Junio C Hamano, git 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 22:01 ` Johannes Schindelin @ 2006-04-09 22:22 ` Timo Hirvonen 2006-04-10 8:10 ` Johannes Schindelin 2006-04-09 23:51 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Timo Hirvonen @ 2006-04-09 22:22 UTC (permalink / raw) To: git; +Cc: torvalds, junkio, git 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 22:22 ` Timo Hirvonen @ 2006-04-10 8:10 ` Johannes Schindelin 0 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2006-04-10 8:10 UTC (permalink / raw) To: Timo Hirvonen; +Cc: git, junkio 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 22:01 ` Johannes Schindelin 2006-04-09 22:22 ` Timo Hirvonen @ 2006-04-09 23:51 ` Junio C Hamano 2006-04-10 0:06 ` Linus Torvalds 2006-04-10 8:22 ` Johannes Schindelin 1 sibling, 2 replies; 22+ messages in thread From: Junio C Hamano @ 2006-04-09 23:51 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Linus Torvalds, git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 23:51 ` Junio C Hamano @ 2006-04-10 0:06 ` Linus Torvalds 2006-04-10 8:22 ` Johannes Schindelin 1 sibling, 0 replies; 22+ messages in thread From: Linus Torvalds @ 2006-04-10 0:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 23:51 ` Junio C Hamano 2006-04-10 0:06 ` Linus Torvalds @ 2006-04-10 8:22 ` Johannes Schindelin 1 sibling, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2006-04-10 8:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: Linus Torvalds, git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 21:13 ` Johannes Schindelin 2006-04-09 22:01 ` Johannes Schindelin @ 2006-04-11 0:04 ` Randal L. Schwartz 2006-04-11 0:12 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Randal L. Schwartz @ 2006-04-11 0:04 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Linus Torvalds, Junio C Hamano, git >>>>> "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! ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-11 0:04 ` Randal L. Schwartz @ 2006-04-11 0:12 ` Junio C Hamano 2006-04-11 0:31 ` Petr Baudis 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2006-04-11 0:12 UTC (permalink / raw) To: Randal L. Schwartz; +Cc: git 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) ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-11 0:12 ` Junio C Hamano @ 2006-04-11 0:31 ` Petr Baudis 2006-04-11 0:50 ` Randal L. Schwartz 0 siblings, 1 reply; 22+ messages in thread From: Petr Baudis @ 2006-04-11 0:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: Randal L. Schwartz, git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-11 0:31 ` Petr Baudis @ 2006-04-11 0:50 ` Randal L. Schwartz 0 siblings, 0 replies; 22+ messages in thread From: Randal L. Schwartz @ 2006-04-11 0:50 UTC (permalink / raw) To: Petr Baudis; +Cc: Junio C Hamano, git >>>>> "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! ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-09 16:53 ` Linus Torvalds 2006-04-09 18:46 ` Junio C Hamano @ 2006-04-10 23:42 ` Junio C Hamano 2006-04-10 23:43 ` [PATCH] tree-diff: do not assume we use only one pathspec Junio C Hamano ` (2 more replies) 1 sibling, 3 replies; 22+ messages in thread From: Junio C Hamano @ 2006-04-10 23:42 UTC (permalink / raw) To: Linus Torvalds; +Cc: git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] tree-diff: do not assume we use only one pathspec 2006-04-10 23:42 ` Junio C Hamano @ 2006-04-10 23:43 ` Junio C Hamano 2006-04-10 23:44 ` [PATCH] git log --full-diff Junio C Hamano 2006-04-10 23:54 ` [PATCH] git log [diff-tree options] Linus Torvalds 2 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2006-04-10 23:43 UTC (permalink / raw) To: git; +Cc: Linus Torvalds 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] git log --full-diff 2006-04-10 23:42 ` Junio C Hamano 2006-04-10 23:43 ` [PATCH] tree-diff: do not assume we use only one pathspec Junio C Hamano @ 2006-04-10 23:44 ` Junio C Hamano 2006-04-10 23:54 ` [PATCH] git log [diff-tree options] Linus Torvalds 2 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2006-04-10 23:44 UTC (permalink / raw) To: git; +Cc: Linus Torvalds 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] git log [diff-tree options]... 2006-04-10 23:42 ` Junio C Hamano 2006-04-10 23:43 ` [PATCH] tree-diff: do not assume we use only one pathspec Junio C Hamano 2006-04-10 23:44 ` [PATCH] git log --full-diff Junio C Hamano @ 2006-04-10 23:54 ` Linus Torvalds 2 siblings, 0 replies; 22+ messages in thread From: Linus Torvalds @ 2006-04-10 23:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2006-04-11 0:50 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-04-09 9:04 [PATCH] git log [diff-tree options] Junio C Hamano 2006-04-09 9:16 ` Junio C Hamano 2006-04-09 16:53 ` Linus Torvalds 2006-04-09 18:46 ` Junio C Hamano 2006-04-09 19:02 ` Linus Torvalds 2006-04-09 19:08 ` Junio C Hamano 2006-04-09 19:26 ` Linus Torvalds 2006-04-09 21:13 ` Johannes Schindelin 2006-04-09 22:01 ` Johannes Schindelin 2006-04-09 22:22 ` Timo Hirvonen 2006-04-10 8:10 ` Johannes Schindelin 2006-04-09 23:51 ` Junio C Hamano 2006-04-10 0:06 ` Linus Torvalds 2006-04-10 8:22 ` Johannes Schindelin 2006-04-11 0:04 ` Randal L. Schwartz 2006-04-11 0:12 ` Junio C Hamano 2006-04-11 0:31 ` Petr Baudis 2006-04-11 0:50 ` Randal L. Schwartz 2006-04-10 23:42 ` Junio C Hamano 2006-04-10 23:43 ` [PATCH] tree-diff: do not assume we use only one pathspec Junio C Hamano 2006-04-10 23:44 ` [PATCH] git log --full-diff Junio C Hamano 2006-04-10 23:54 ` [PATCH] git log [diff-tree options] Linus Torvalds
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).