From db89665fbf86d4e0166b2f252a939ed8bf6782fe Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Apr 2006 12:42:36 -0700 Subject: [PATCH 1/6] rev-list --header: output format fix Initial fix prepared by Johannes, but I did it slightly differently. Signed-off-by: Junio C Hamano --- rev-list.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rev-list.c b/rev-list.c index 000f27a8fe..d3c0dd90e3 100644 --- a/rev-list.c +++ b/rev-list.c @@ -326,6 +326,9 @@ int main(int argc, const char **argv) else revs.header_prefix = "commit "; } + else if (revs.verbose_header) + /* Only --header was specified */ + revs.commit_format = CMIT_FMT_RAW; list = revs.commits; From 9153983310a169a340bd1023dccafd80b70b05bc Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 17 Apr 2006 11:59:32 -0700 Subject: [PATCH 2/6] Log message printout cleanups On Sun, 16 Apr 2006, Junio C Hamano wrote: > > In the mid-term, I am hoping we can drop the generate_header() > callchain _and_ the custom code that formats commit log in-core, > found in cmd_log_wc(). Ok, this was nastier than expected, just because the dependencies between the different log-printing stuff were absolutely _everywhere_, but here's a patch that does exactly that. The patch is not very easy to read, and the "--patch-with-stat" thing is still broken (it does not call the "show_log()" thing properly for merges). That's not a new bug. In the new world order it _should_ do something like if (rev->logopt) show_log(rev, rev->logopt, "---\n"); but it doesn't. I haven't looked at the --with-stat logic, so I left it alone. That said, this patch removes more lines than it adds, and in particular, the "cmd_log_wc()" loop is now a very clean: while ((commit = get_revision(rev)) != NULL) { log_tree_commit(rev, commit); free(commit->buffer); commit->buffer = NULL; } so it doesn't get much prettier than this. All the complexity is entirely hidden in log-tree.c, and any code that needs to flush the log literally just needs to do the "if (rev->logopt) show_log(...)" incantation. I had to make the combined_diff() logic take a "struct rev_info" instead of just a "struct diff_options", but that part is pretty clean. This does change "git whatchanged" from using "diff-tree" as the commit descriptor to "commit", and I changed one of the tests to reflect that new reality. Otherwise everything still passes, and my other tests look fine too. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- combine-diff.c | 47 ++++++------- diff-files.c | 27 +++---- diff-tree.c | 1 + diff.h | 8 +-- git.c | 57 +-------------- log-tree.c | 167 ++++++++++++++++++++++++-------------------- log-tree.h | 5 ++ rev-list.c | 9 +-- revision.c | 3 - revision.h | 8 +-- t/t1200-tutorial.sh | 4 +- 11 files changed, 151 insertions(+), 185 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index 9bd27f82ec..b4fa9c9f68 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -5,6 +5,7 @@ #include "diffcore.h" #include "quote.h" #include "xdiff-interface.h" +#include "log-tree.h" static int uninteresting(struct diff_filepair *p) { @@ -585,9 +586,9 @@ static void reuse_combine_diff(struct sline *sline, unsigned long cnt, } static int show_patch_diff(struct combine_diff_path *elem, int num_parent, - int dense, const char *header, - struct diff_options *opt) + int dense, struct rev_info *rev) { + struct diff_options *opt = &rev->diffopt; unsigned long result_size, cnt, lno; char *result, *cp, *ep; struct sline *sline; /* survived lines */ @@ -689,10 +690,8 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent, if (show_hunks || mode_differs || working_tree_file) { const char *abb; - if (header) { - shown_header++; - printf("%s%c", header, opt->line_termination); - } + if (rev->loginfo) + show_log(rev, rev->loginfo, "\n"); printf("diff --%s ", dense ? "cc" : "combined"); if (quote_c_style(elem->path, NULL, NULL, 0)) quote_c_style(elem->path, NULL, stdout, 0); @@ -750,8 +749,9 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent, #define COLONS "::::::::::::::::::::::::::::::::" -static void show_raw_diff(struct combine_diff_path *p, int num_parent, const char *header, struct diff_options *opt) +static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev) { + struct diff_options *opt = &rev->diffopt; int i, offset, mod_type = 'A'; const char *prefix; int line_termination, inter_name_termination; @@ -761,8 +761,8 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, const cha if (!line_termination) inter_name_termination = 0; - if (header) - printf("%s%c", header, line_termination); + if (rev->loginfo) + show_log(rev, rev->loginfo, "\n"); for (i = 0; i < num_parent; i++) { if (p->parent[i].mode) @@ -810,31 +810,31 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, const cha } } -int show_combined_diff(struct combine_diff_path *p, +void show_combined_diff(struct combine_diff_path *p, int num_parent, int dense, - const char *header, - struct diff_options *opt) + struct rev_info *rev) { + struct diff_options *opt = &rev->diffopt; if (!p->len) - return 0; + return; switch (opt->output_format) { case DIFF_FORMAT_RAW: case DIFF_FORMAT_NAME_STATUS: case DIFF_FORMAT_NAME: - show_raw_diff(p, num_parent, header, opt); - return 1; + show_raw_diff(p, num_parent, rev); + return; default: case DIFF_FORMAT_PATCH: - return show_patch_diff(p, num_parent, dense, header, opt); + show_patch_diff(p, num_parent, dense, rev); } } -const char *diff_tree_combined_merge(const unsigned char *sha1, - const char *header, int dense, - struct diff_options *opt) +void diff_tree_combined_merge(const unsigned char *sha1, + int dense, struct rev_info *rev) { + struct diff_options *opt = &rev->diffopt; struct commit *commit = lookup_commit(sha1); struct diff_options diffopts; struct commit_list *parents; @@ -874,17 +874,13 @@ const char *diff_tree_combined_merge(const unsigned char *sha1, int saved_format = opt->output_format; opt->output_format = DIFF_FORMAT_RAW; for (p = paths; p; p = p->next) { - if (show_combined_diff(p, num_parent, dense, - header, opt)) - header = NULL; + show_combined_diff(p, num_parent, dense, rev); } opt->output_format = saved_format; putchar(opt->line_termination); } for (p = paths; p; p = p->next) { - if (show_combined_diff(p, num_parent, dense, - header, opt)) - header = NULL; + show_combined_diff(p, num_parent, dense, rev); } } @@ -894,5 +890,4 @@ const char *diff_tree_combined_merge(const unsigned char *sha1, paths = paths->next; free(tmp); } - return header; } diff --git a/diff-files.c b/diff-files.c index 3e7f5f105b..ffbef48b2e 100644 --- a/diff-files.c +++ b/diff-files.c @@ -5,12 +5,14 @@ */ #include "cache.h" #include "diff.h" +#include "commit.h" +#include "revision.h" static const char diff_files_usage[] = "git-diff-files [-q] [-0/-1/2/3 |-c|--cc] [] [...]" COMMON_DIFF_OPTIONS_HELP; -static struct diff_options diff_options; +static struct rev_info rev; static int silent = 0; static int diff_unmerged_stage = 2; static int combine_merges = 0; @@ -18,12 +20,12 @@ static int dense_combined_merges = 0; static void show_unmerge(const char *path) { - diff_unmerge(&diff_options, path); + diff_unmerge(&rev.diffopt, path); } static void show_file(int pfx, struct cache_entry *ce) { - diff_addremove(&diff_options, pfx, ntohl(ce->ce_mode), + diff_addremove(&rev.diffopt, pfx, ntohl(ce->ce_mode), ce->sha1, ce->name, NULL); } @@ -31,7 +33,7 @@ static void show_modified(int oldmode, int mode, const unsigned char *old_sha1, const unsigned char *sha1, char *path) { - diff_change(&diff_options, oldmode, mode, old_sha1, sha1, path, NULL); + diff_change(&rev.diffopt, oldmode, mode, old_sha1, sha1, path, NULL); } int main(int argc, const char **argv) @@ -41,7 +43,7 @@ int main(int argc, const char **argv) int entries, i; git_config(git_diff_config); - diff_setup(&diff_options); + diff_setup(&rev.diffopt); while (1 < argc && argv[1][0] == '-') { if (!strcmp(argv[1], "--")) { argv++; @@ -74,7 +76,7 @@ int main(int argc, const char **argv) dense_combined_merges = combine_merges = 1; else { int diff_opt_cnt; - diff_opt_cnt = diff_opt_parse(&diff_options, + diff_opt_cnt = diff_opt_parse(&rev.diffopt, argv+1, argc-1); if (diff_opt_cnt < 0) usage(diff_files_usage); @@ -89,13 +91,13 @@ int main(int argc, const char **argv) argv++; argc--; } if (dense_combined_merges) - diff_options.output_format = DIFF_FORMAT_PATCH; + rev.diffopt.output_format = DIFF_FORMAT_PATCH; /* Find the directory, and set up the pathspec */ pathspec = get_pathspec(prefix, argv + 1); entries = read_cache(); - if (diff_setup_done(&diff_options) < 0) + if (diff_setup_done(&rev.diffopt) < 0) usage(diff_files_usage); /* At this point, if argc == 1, then we are doing everything. @@ -167,8 +169,7 @@ int main(int argc, const char **argv) if (combine_merges && num_compare_stages == 2) { show_combined_diff(&combine.p, 2, dense_combined_merges, - NULL, - &diff_options); + &rev); free(combine.p.path); continue; } @@ -194,7 +195,7 @@ int main(int argc, const char **argv) continue; } changed = ce_match_stat(ce, &st, 0); - if (!changed && !diff_options.find_copies_harder) + if (!changed && !rev.diffopt.find_copies_harder) continue; oldmode = ntohl(ce->ce_mode); @@ -207,7 +208,7 @@ int main(int argc, const char **argv) ce->sha1, (changed ? null_sha1 : ce->sha1), ce->name); } - diffcore_std(&diff_options); - diff_flush(&diff_options); + diffcore_std(&rev.diffopt); + diff_flush(&rev.diffopt); return 0; } diff --git a/diff-tree.c b/diff-tree.c index e578798537..7207867a74 100644 --- a/diff-tree.c +++ b/diff-tree.c @@ -71,6 +71,7 @@ int main(int argc, const char **argv) nr_sha1 = 0; init_revisions(opt); opt->abbrev = 0; + opt->diff = 1; argc = setup_revisions(argc, argv, opt, NULL); while (--argc > 0) { diff --git a/diff.h b/diff.h index f783baef14..52fff6643f 100644 --- a/diff.h +++ b/diff.h @@ -6,6 +6,7 @@ #include "tree-walk.h" +struct rev_info; struct diff_options; typedef void (*change_fn_t)(struct diff_options *options, @@ -70,11 +71,10 @@ struct combine_diff_path { (sizeof(struct combine_diff_path) + \ sizeof(struct combine_diff_parent) * (n) + (l) + 1) -extern int show_combined_diff(struct combine_diff_path *elem, int num_parent, - int dense, const char *header, - struct diff_options *); +extern void show_combined_diff(struct combine_diff_path *elem, int num_parent, + int dense, struct rev_info *); -extern const char *diff_tree_combined_merge(const unsigned char *sha1, const char *, int, struct diff_options *opt); +extern void diff_tree_combined_merge(const unsigned char *sha1, int, struct rev_info *); extern void diff_addremove(struct diff_options *, int addremove, diff --git a/git.c b/git.c index fc4e429278..016fa30d37 100644 --- a/git.c +++ b/git.c @@ -282,75 +282,22 @@ static int cmd_log_wc(int argc, const char **argv, char **envp, struct rev_info *rev) { struct commit *commit; - char *buf = xmalloc(LOGSIZE); - const char *commit_prefix = "commit "; - int shown = 0; rev->abbrev = DEFAULT_ABBREV; rev->commit_format = CMIT_FMT_DEFAULT; + rev->verbose_header = 1; argc = setup_revisions(argc, argv, rev, "HEAD"); if (argc > 1) die("unrecognized argument: %s", argv[1]); - if (rev->commit_format == CMIT_FMT_ONELINE) - commit_prefix = ""; prepare_revision_walk(rev); setup_pager(); while ((commit = get_revision(rev)) != NULL) { - unsigned long ofs = 0; - - if (shown && rev->diff && - rev->commit_format != CMIT_FMT_ONELINE) - putchar('\n'); - - ofs = sprintf(buf, "%s", commit_prefix); - if (rev->abbrev_commit && rev->abbrev) - ofs += sprintf(buf + ofs, "%s", - find_unique_abbrev(commit->object.sha1, - rev->abbrev)); - else - ofs += sprintf(buf + ofs, "%s", - sha1_to_hex(commit->object.sha1)); - if (rev->parents) { - struct commit_list *parents = commit->parents; - while (parents) { - struct object *o = &(parents->item->object); - parents = parents->next; - if (o->flags & TMP_MARK) - continue; - ofs += sprintf(buf + ofs, " %s", - sha1_to_hex(o->sha1)); - o->flags |= TMP_MARK; - } - /* TMP_MARK is a general purpose flag that can - * be used locally, but the user should clean - * things up after it is done with them. - */ - for (parents = commit->parents; - parents; - parents = parents->next) - parents->item->object.flags &= ~TMP_MARK; - } - buf[ofs++] = - (rev->commit_format == CMIT_FMT_ONELINE) ? ' ' : '\n'; - ofs += pretty_print_commit(rev->commit_format, commit, ~0, - buf + ofs, - LOGSIZE - ofs - 20, - rev->abbrev); - - if (rev->diff) { - rev->use_precomputed_header = buf; - strcpy(buf + ofs, "\n---\n"); - log_tree_commit(rev, commit); - } - else - printf("%s\n", buf); - shown = 1; + log_tree_commit(rev, commit); free(commit->buffer); commit->buffer = NULL; } - free(buf); return 0; } diff --git a/log-tree.c b/log-tree.c index af36f702e1..c0a4432022 100644 --- a/log-tree.c +++ b/log-tree.c @@ -1,11 +1,51 @@ + #include "cache.h" #include "diff.h" #include "commit.h" #include "log-tree.h" +void show_log(struct rev_info *opt, struct log_info *log, const char *sep) +{ + static char this_header[16384]; + struct commit *commit = log->commit, *parent = log->parent; + int abbrev = opt->diffopt.abbrev; + int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40; + int len; + + opt->loginfo = NULL; + if (!opt->verbose_header) { + puts(sha1_to_hex(commit->object.sha1)); + return; + } + + /* + * Whitespace between commit messages, unless we are oneline + */ + if (opt->shown_one && opt->commit_format != CMIT_FMT_ONELINE) + putchar('\n'); + opt->shown_one = 1; + + /* + * Print header line of header.. + */ + printf("%s%s", + opt->commit_format == CMIT_FMT_ONELINE ? "" : "commit ", + diff_unique_abbrev(commit->object.sha1, abbrev_commit)); + if (parent) + printf(" (from %s)", diff_unique_abbrev(parent->object.sha1, abbrev_commit)); + putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n'); + + /* + * And then the pretty-printed message itself + */ + len = pretty_print_commit(opt->commit_format, commit, ~0u, this_header, sizeof(this_header), abbrev); + printf("%s%s", this_header, sep); +} + int log_tree_diff_flush(struct rev_info *opt) { diffcore_std(&opt->diffopt); + if (diff_queue_is_empty()) { int saved_fmt = opt->diffopt.output_format; opt->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT; @@ -13,12 +53,9 @@ int log_tree_diff_flush(struct rev_info *opt) opt->diffopt.output_format = saved_fmt; return 0; } - if (opt->header) { - if (!opt->no_commit_id) - printf("%s%c", opt->header, - opt->diffopt.line_termination); - opt->header = NULL; - } + + if (opt->loginfo && !opt->no_commit_id) + show_log(opt, opt->loginfo, "\n"); diff_flush(&opt->diffopt); return 1; } @@ -43,96 +80,78 @@ static int diff_root_tree(struct rev_info *opt, return retval; } -static const char *get_header(struct rev_info *opt, - const unsigned char *commit_sha1, - const unsigned char *parent_sha1, - const struct commit *commit) -{ - static char this_header[16384]; - int offset; - unsigned long len; - int abbrev = opt->diffopt.abbrev; - const char *msg = commit->buffer; - - if (opt->use_precomputed_header) - return opt->use_precomputed_header; - - if (!opt->verbose_header) - return sha1_to_hex(commit_sha1); - - len = strlen(msg); - - offset = sprintf(this_header, "%s%s ", - opt->header_prefix, - diff_unique_abbrev(commit_sha1, abbrev)); - if (commit_sha1 != parent_sha1) - offset += sprintf(this_header + offset, "(from %s)\n", - parent_sha1 - ? diff_unique_abbrev(parent_sha1, abbrev) - : "root"); - else - offset += sprintf(this_header + offset, "(from parents)\n"); - offset += pretty_print_commit(opt->commit_format, commit, len, - this_header + offset, - sizeof(this_header) - offset, abbrev); - return this_header; -} - -static const char *generate_header(struct rev_info *opt, - const unsigned char *commit_sha1, - const unsigned char *parent_sha1, - const struct commit *commit) -{ - const char *header = get_header(opt, commit_sha1, parent_sha1, commit); - - if (opt->always_show_header) { - puts(header); - header = NULL; - } - return header; -} - static int do_diff_combined(struct rev_info *opt, struct commit *commit) { unsigned const char *sha1 = commit->object.sha1; - opt->header = generate_header(opt, sha1, sha1, commit); - opt->header = diff_tree_combined_merge(sha1, opt->header, - opt->dense_combined_merges, - &opt->diffopt); - if (!opt->header && opt->verbose_header) - opt->header_prefix = "\ndiff-tree "; - return 0; + diff_tree_combined_merge(sha1, opt->dense_combined_merges, opt); + return !opt->loginfo; } -int log_tree_commit(struct rev_info *opt, struct commit *commit) +/* + * Show the diff of a commit. + * + * Return true if we printed any log info messages + */ +static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log_info *log) { + int showed_log; struct commit_list *parents; unsigned const char *sha1 = commit->object.sha1; + if (!opt->diff) + return 0; + /* Root commit? */ - if (opt->show_root_diff && !commit->parents) { - opt->header = generate_header(opt, sha1, NULL, commit); - diff_root_tree(opt, sha1, ""); + parents = commit->parents; + if (!parents) { + if (opt->show_root_diff) + diff_root_tree(opt, sha1, ""); + return !opt->loginfo; } /* More than one parent? */ - if (commit->parents && commit->parents->next) { + if (parents && parents->next) { if (opt->ignore_merges) return 0; else if (opt->combine_merges) return do_diff_combined(opt, commit); + + /* If we show individual diffs, show the parent info */ + log->parent = parents->item; } - for (parents = commit->parents; parents; parents = parents->next) { + showed_log = 0; + for (;;) { struct commit *parent = parents->item; - unsigned const char *psha1 = parent->object.sha1; - opt->header = generate_header(opt, sha1, psha1, commit); - diff_tree_sha1(psha1, sha1, "", &opt->diffopt); - log_tree_diff_flush(opt); - if (!opt->header && opt->verbose_header) - opt->header_prefix = "\ndiff-tree "; + diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt); + log_tree_diff_flush(opt); + + showed_log |= !opt->loginfo; + + /* Set up the log info for the next parent, if any.. */ + parents = parents->next; + if (!parents) + break; + log->parent = parents->item; + opt->loginfo = log; } + return showed_log; +} + +int log_tree_commit(struct rev_info *opt, struct commit *commit) +{ + struct log_info log; + + log.commit = commit; + log.parent = NULL; + opt->loginfo = &log; + + if (!log_tree_diff(opt, commit, &log) && opt->loginfo && opt->always_show_header) { + log.parent = NULL; + show_log(opt, opt->loginfo, ""); + } + opt->loginfo = NULL; return 0; } diff --git a/log-tree.h b/log-tree.h index 91a909be73..a26e4841ff 100644 --- a/log-tree.h +++ b/log-tree.h @@ -3,9 +3,14 @@ #include "revision.h" +struct log_info { + struct commit *commit, *parent; +}; + void init_log_tree_opt(struct rev_info *); int log_tree_diff_flush(struct rev_info *); int log_tree_commit(struct rev_info *, struct commit *); int log_tree_opt_parse(struct rev_info *, const char **, int); +void show_log(struct rev_info *opt, struct log_info *log, const char *sep); #endif diff --git a/rev-list.c b/rev-list.c index d3c0dd90e3..a4d72af6e0 100644 --- a/rev-list.c +++ b/rev-list.c @@ -41,13 +41,14 @@ struct rev_info revs; static int bisect_list = 0; static int show_timestamp = 0; static int hdr_termination = 0; +static const char *header_prefix; static void show_commit(struct commit *commit) { if (show_timestamp) printf("%lu ", commit->date); - if (*revs.header_prefix) - fputs(revs.header_prefix, stdout); + if (header_prefix) + fputs(header_prefix, stdout); if (commit->object.flags & BOUNDARY) putchar('-'); if (revs.abbrev_commit && revs.abbrev) @@ -322,9 +323,9 @@ int main(int argc, const char **argv) /* The command line has a --pretty */ hdr_termination = '\n'; if (revs.commit_format == CMIT_FMT_ONELINE) - revs.header_prefix = ""; + header_prefix = ""; else - revs.header_prefix = "commit "; + header_prefix = "commit "; } else if (revs.verbose_header) /* Only --header was specified */ diff --git a/revision.c b/revision.c index f8fb028855..5abec18bc1 100644 --- a/revision.c +++ b/revision.c @@ -498,7 +498,6 @@ void init_revisions(struct rev_info *revs) revs->topo_setter = topo_sort_default_setter; revs->topo_getter = topo_sort_default_getter; - revs->header_prefix = ""; revs->commit_format = CMIT_FMT_DEFAULT; diff_setup(&revs->diffopt); @@ -675,12 +674,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch } if (!strcmp(arg, "-v")) { revs->verbose_header = 1; - revs->header_prefix = "diff-tree "; continue; } if (!strncmp(arg, "--pretty", 8)) { revs->verbose_header = 1; - revs->header_prefix = "diff-tree "; revs->commit_format = get_commit_format(arg+8); continue; } diff --git a/revision.h b/revision.h index 74dc5ef179..05f658a214 100644 --- a/revision.h +++ b/revision.h @@ -10,6 +10,7 @@ #define ADDED (1u<<6) /* Parents already parsed and added? */ struct rev_info; +struct log_info; typedef void (prune_fn_t)(struct rev_info *revs, struct commit *commit); @@ -51,12 +52,11 @@ struct rev_info { always_show_header:1; /* Format info */ - unsigned int abbrev_commit:1; + unsigned int shown_one:1, + abbrev_commit:1; unsigned int abbrev; enum cmit_fmt commit_format; - const char *header_prefix; - const char *header; - const char *use_precomputed_header; + struct log_info *loginfo; /* special limits */ int max_count; diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh index 10024133e3..16b3ea9157 100755 --- a/t/t1200-tutorial.sh +++ b/t/t1200-tutorial.sh @@ -49,7 +49,7 @@ test_expect_success 'git diff HEAD' 'cmp diff.expect diff.output' #test_expect_success 'git-read-tree --reset HEAD' "git-read-tree --reset HEAD ; test \"hello: needs update\" = \"$(git-update-index --refresh)\"" cat > whatchanged.expect << EOF -diff-tree VARIABLE (from root) +commit VARIABLE Author: VARIABLE Date: VARIABLE @@ -72,7 +72,7 @@ index 0000000..557db03 EOF git-whatchanged -p --root | \ - sed -e "1s/^\(.\{10\}\).\{40\}/\1VARIABLE/" \ + sed -e "1s/^\(.\{7\}\).\{40\}/\1VARIABLE/" \ -e "2,3s/^\(.\{8\}\).*$/\1VARIABLE/" \ > whatchanged.output test_expect_success 'git-whatchanged -p --root' 'cmp whatchanged.expect whatchanged.output' From eab144ac49c18d981261c2d0ba964d6380d9f1da Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 17 Apr 2006 16:59:42 -0700 Subject: [PATCH 3/6] Log message printout cleanups (#2) Here's a further patch on top of the previous one with cosmetic improvements (no "real" code changes, just trivial updates): - it gets the "---" before a diffstat right, including for the combined merge case. Righ now the logic is that we always use "---" when we have a diffstat, and an empty line otherwise. That's how I visually prefer it, but hey, it can be tweaked later. - I made "diff --cc/combined" add the "---/+++" header lines too. The thing won't be mistaken for a valid diff, since the "@@" lines have too many "@" characters (three or more), but it just makes it visually match a real diff, which at least to me makes a big difference in readability. Without them, it just looks very "wrong". I guess I should have taken the filename from each individual entry (and had one "---" file per parent), but I didn't even bother to try to see how that works, so this was the simple thing. With this, doing a git log --cc --patch-with-stat looks quite readable, I think. The only nagging issue - as far as I'm concerned - is that diffstats for merges are pretty questionable the way they are done now. I suspect it would be better to just have the _first_ diffstat, and always make the merge diffstat be the one for "result against first parent". Signed-off-by: Junio C Hamano --- combine-diff.c | 22 ++++++++++++++++------ log-tree.c | 3 +-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index b4fa9c9f68..aef9006443 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -585,6 +585,16 @@ static void reuse_combine_diff(struct sline *sline, unsigned long cnt, sline->p_lno[i] = sline->p_lno[j]; } +static void dump_quoted_path(const char *prefix, const char *path) +{ + fputs(prefix, stdout); + if (quote_c_style(path, NULL, NULL, 0)) + quote_c_style(path, NULL, stdout, 0); + else + printf("%s", path); + putchar('\n'); +} + static int show_patch_diff(struct combine_diff_path *elem, int num_parent, int dense, struct rev_info *rev) { @@ -692,12 +702,7 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent, if (rev->loginfo) show_log(rev, rev->loginfo, "\n"); - printf("diff --%s ", dense ? "cc" : "combined"); - if (quote_c_style(elem->path, NULL, NULL, 0)) - quote_c_style(elem->path, NULL, stdout, 0); - else - printf("%s", elem->path); - putchar('\n'); + dump_quoted_path(dense ? "diff --cc " : "diff --combined ", elem->path); printf("index "); for (i = 0; i < num_parent; i++) { abb = find_unique_abbrev(elem->parent[i].sha1, @@ -728,6 +733,8 @@ static int show_patch_diff(struct combine_diff_path *elem, int num_parent, } putchar('\n'); } + dump_quoted_path("--- a/", elem->path); + dump_quoted_path("+++ b/", elem->path); dump_sline(sline, cnt, num_parent); } free(result); @@ -861,6 +868,9 @@ void diff_tree_combined_merge(const unsigned char *sha1, &diffopts); diffcore_std(&diffopts); paths = intersect_paths(paths, i, num_parent); + + if (diffopts.with_stat && rev->loginfo) + show_log(rev, rev->loginfo, "---\n"); diff_flush(&diffopts); } diff --git a/log-tree.c b/log-tree.c index c0a4432022..9e5416427a 100644 --- a/log-tree.c +++ b/log-tree.c @@ -1,4 +1,3 @@ - #include "cache.h" #include "diff.h" #include "commit.h" @@ -55,7 +54,7 @@ int log_tree_diff_flush(struct rev_info *opt) } if (opt->loginfo && !opt->no_commit_id) - show_log(opt, opt->loginfo, "\n"); + show_log(opt, opt->loginfo, opt->diffopt.with_stat ? "---\n" : "\n"); diff_flush(&opt->diffopt); return 1; } From a4d34e2db5565e6b75f79f9d3938aa9151e72e44 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 17 Apr 2006 17:43:40 -0700 Subject: [PATCH 4/6] Log message printout cleanups (#3): fix --pretty=oneline This option is very special, since pretty_print_commit() will _remove_ the newline at the end of it, so we want to have an extra separator between the things. I added a honking big comment this time, so that (a) I don't forget this _again_ (I broke "oneline" several times during this printout cleanup), and so that people can understand _why_ the code does what it does. Now, arguably the alternate fix is to always have the '\n' at the end in pretty-print-commit, but git-rev-list depends on the current behaviour (but we could have git-rev-list remove it, whatever). With the big comment, the code hopefully doesn't get broken again. And now things like git log --pretty=oneline --cc --patch-with-stat works (even if that is admittedly a totally insane combination: if you want the patch, having the "oneline" log format is just crazy, but hey, it _works_. Even insane people are people). Signed-off-by: Junio C Hamano --- log-tree.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/log-tree.c b/log-tree.c index 9e5416427a..9634c4677f 100644 --- a/log-tree.c +++ b/log-tree.c @@ -9,6 +9,7 @@ void show_log(struct rev_info *opt, struct log_info *log, const char *sep) struct commit *commit = log->commit, *parent = log->parent; int abbrev = opt->diffopt.abbrev; int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40; + const char *extra; int len; opt->loginfo = NULL; @@ -18,8 +19,17 @@ void show_log(struct rev_info *opt, struct log_info *log, const char *sep) } /* - * Whitespace between commit messages, unless we are oneline + * The "oneline" format has several special cases: + * - The pretty-printed commit lacks a newline at the end + * of the buffer, but we do want to make sure that we + * have a newline there. If the separator isn't already + * a newline, add an extra one. + * - unlike other log messages, the one-line format does + * not have an empty line between entries. */ + extra = ""; + if (*sep != '\n' && opt->commit_format == CMIT_FMT_ONELINE) + extra = "\n"; if (opt->shown_one && opt->commit_format != CMIT_FMT_ONELINE) putchar('\n'); opt->shown_one = 1; @@ -38,7 +48,7 @@ void show_log(struct rev_info *opt, struct log_info *log, const char *sep) * And then the pretty-printed message itself */ len = pretty_print_commit(opt->commit_format, commit, ~0u, this_header, sizeof(this_header), abbrev); - printf("%s%s", this_header, sep); + printf("%s%s%s", this_header, extra, sep); } int log_tree_diff_flush(struct rev_info *opt) From b073f26b256cded6252bafd34982eb6f69d2a4b6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Apr 2006 21:47:35 -0700 Subject: [PATCH 5/6] git.c: LOGSIZE is unused after log printing cleanup. Signed-off-by: Junio C Hamano --- git.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/git.c b/git.c index 016fa30d37..0be14bb487 100644 --- a/git.c +++ b/git.c @@ -276,8 +276,6 @@ static int cmd_help(int argc, const char **argv, char **envp) return 0; } -#define LOGSIZE (65536) - static int cmd_log_wc(int argc, const char **argv, char **envp, struct rev_info *rev) { From 965f803c323bb72a9dedbbc8f7ba00bbadb6cf58 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Apr 2006 22:53:03 -0700 Subject: [PATCH 6/6] combine-diff: show diffstat with the first parent. Asking for stat (either with --stat or --patch-with-stat) gives you diffstat for the first parent, even under combine-diff. While the combined patch is useful to highlight the complexity and interaction of the parts touched by all branches when reviewing a merge commit, diffstat is a tool to assess the extent of damage the merge brings in, and showing stat with the first parent is more sensible than clever per-parent diffstat. Signed-off-by: Junio C Hamano --- combine-diff.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index aef9006443..27f6f57f3a 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -831,10 +831,11 @@ void show_combined_diff(struct combine_diff_path *p, case DIFF_FORMAT_NAME: show_raw_diff(p, num_parent, rev); return; - - default: case DIFF_FORMAT_PATCH: show_patch_diff(p, num_parent, dense, rev); + return; + default: + return; } } @@ -847,10 +848,13 @@ void diff_tree_combined_merge(const unsigned char *sha1, struct commit_list *parents; struct combine_diff_path *p, *paths = NULL; int num_parent, i, num_paths; + int do_diffstat; + do_diffstat = (opt->output_format == DIFF_FORMAT_DIFFSTAT || + opt->with_stat); diffopts = *opt; - diffopts.output_format = DIFF_FORMAT_NO_OUTPUT; diffopts.with_raw = 0; + diffopts.with_stat = 0; diffopts.recursive = 1; /* count parents */ @@ -864,14 +868,24 @@ void diff_tree_combined_merge(const unsigned char *sha1, parents; parents = parents->next, i++) { struct commit *parent = parents->item; + /* show stat against the first parent even + * when doing combined diff. + */ + if (i == 0 && do_diffstat) + diffopts.output_format = DIFF_FORMAT_DIFFSTAT; + else + diffopts.output_format = DIFF_FORMAT_NO_OUTPUT; diff_tree_sha1(parent->object.sha1, commit->object.sha1, "", &diffopts); diffcore_std(&diffopts); paths = intersect_paths(paths, i, num_parent); - if (diffopts.with_stat && rev->loginfo) - show_log(rev, rev->loginfo, "---\n"); + if (do_diffstat && rev->loginfo) + show_log(rev, rev->loginfo, + opt->with_stat ? "---\n" : "\n"); diff_flush(&diffopts); + if (opt->with_stat) + putchar('\n'); } /* find out surviving paths */