From cd2bdc5309461034e5cc58e1d3e87535ed9e093b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 14 Apr 2006 16:52:13 -0700 Subject: [PATCH 01/17] Common option parsing for "git log --diff" and friends This basically does a few things that are sadly somewhat interdependent, and nontrivial to split out - get rid of "struct log_tree_opt" The fields in "log_tree_opt" are moved into "struct rev_info", and all users of log_tree_opt are changed to use the rev_info struct instead. - add the parsing for the log_tree_opt arguments to "setup_revision()" - make setup_revision set a flag (revs->diff) if the diff-related arguments were used. This allows "git log" to decide whether it wants to show diffs or not. - make setup_revision() also initialize the diffopt part of rev_info (which we had from before, but we just didn't initialize it) - make setup_revision() do all the "finishing touches" on it all (it will do the proper flag combination logic, and call "diff_setup_done()") Now, that was the easy and straightforward part. The slightly more involved part is that some of the programs that want to use the new-and-improved rev_info parsing don't actually want _commits_, they may want tree'ish arguments instead. That meant that I had to change setup_revision() to parse the arguments not into the "revs->commits" list, but into the "revs->pending_objects" list. Then, when we do "prepare_revision_walk()", we walk that list, and create the sorted commit list from there. This actually cleaned some stuff up, but it's the less obvious part of the patch, and re-organized the "revision.c" logic somewhat. It actually paves the way for splitting argument parsing _entirely_ out of "revision.c", since now the argument parsing really is totally independent of the commit walking: that didn't use to be true, since there was lots of overlap with get_commit_reference() handling etc, now the _only_ overlap is the shared (and trivial) "add_pending_object()" thing. However, I didn't do that file split, just because I wanted the diff itself to be smaller, and show the actual changes more clearly. If this gets accepted, I'll do further cleanups then - that includes the file split, but also using the new infrastructure to do a nicer "git diff" etc. Even in this form, it actually ends up removing more lines than it adds. It's nice to note how simple and straightforward this makes the built-in "git log" command, even though it continues to support all the diff flags too. It doesn't get much simpler that this. I think this is worth merging soonish, because it does allow for future cleanup and even more sharing of code. However, it obviously touches "revision.c", which is subtle. I've tested that it passes all the tests we have, and it passes my "looks sane" detector, but somebody else should also give it a good look-over. [jc: squashed the original and three "oops this too" updates, with another fix-up.] Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- diff-tree.c | 91 +++++++++++++--------------- git.c | 83 ++++---------------------- log-tree.c | 60 ++----------------- log-tree.h | 22 ++----- revision.c | 168 +++++++++++++++++++++++++++++++++++++++++----------- revision.h | 21 ++++++- 6 files changed, 218 insertions(+), 227 deletions(-) diff --git a/diff-tree.c b/diff-tree.c index 2b79dd0a68..54157e40dd 100644 --- a/diff-tree.c +++ b/diff-tree.c @@ -3,7 +3,7 @@ #include "commit.h" #include "log-tree.h" -static struct log_tree_opt log_tree_opt; +static struct rev_info log_tree_opt; static int diff_tree_commit_sha1(const unsigned char *sha1) { @@ -62,47 +62,18 @@ int main(int argc, const char **argv) { int nr_sha1; char line[1000]; - unsigned char sha1[2][20]; - const char *prefix = setup_git_directory(); - static struct log_tree_opt *opt = &log_tree_opt; + struct object *tree1, *tree2; + static struct rev_info *opt = &log_tree_opt; + struct object_list *list; int read_stdin = 0; git_config(git_diff_config); nr_sha1 = 0; - init_log_tree_opt(opt); + argc = setup_revisions(argc, argv, opt, NULL); - for (;;) { - int opt_cnt; - const char *arg; + while (--argc > 0) { + const char *arg = *++argv; - argv++; - argc--; - arg = *argv; - if (!arg) - break; - - if (*arg != '-') { - if (nr_sha1 < 2 && !get_sha1(arg, sha1[nr_sha1])) { - nr_sha1++; - continue; - } - break; - } - - opt_cnt = log_tree_opt_parse(opt, argv, argc); - if (opt_cnt < 0) - usage(diff_tree_usage); - else if (opt_cnt) { - argv += opt_cnt - 1; - argc -= opt_cnt - 1; - continue; - } - - if (!strcmp(arg, "--")) { - argv++; - argc--; - break; - } if (!strcmp(arg, "--stdin")) { read_stdin = 1; continue; @@ -110,18 +81,36 @@ int main(int argc, const char **argv) usage(diff_tree_usage); } - if (opt->combine_merges) - opt->ignore_merges = 0; - - /* We can only do dense combined merges with diff output */ - 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_tree_setup_paths(get_pathspec(prefix, argv), opt); - diff_setup_done(&opt->diffopt); + /* + * NOTE! "setup_revisions()" will have inserted the revisions + * it parsed in reverse order. So if you do + * + * git-diff-tree a b + * + * the commit list will be "b" -> "a" -> NULL, so we reverse + * the order of the objects if the first one is not marked + * UNINTERESTING. + */ + nr_sha1 = 0; + list = opt->pending_objects; + if (list) { + nr_sha1++; + tree1 = list->item; + list = list->next; + if (list) { + nr_sha1++; + tree2 = tree1; + tree1 = list->item; + if (list->next) + usage(diff_tree_usage); + /* Switch them around if the second one was uninteresting.. */ + if (tree2->flags & UNINTERESTING) { + struct object *tmp = tree2; + tree2 = tree1; + tree1 = tmp; + } + } + } switch (nr_sha1) { case 0: @@ -129,10 +118,12 @@ int main(int argc, const char **argv) usage(diff_tree_usage); break; case 1: - diff_tree_commit_sha1(sha1[0]); + diff_tree_commit_sha1(tree1->sha1); break; case 2: - diff_tree_sha1(sha1[0], sha1[1], "", &opt->diffopt); + diff_tree_sha1(tree1->sha1, + tree2->sha1, + "", &opt->diffopt); log_tree_diff_flush(opt); break; } diff --git a/git.c b/git.c index 78ed403ed1..437e9b57d0 100644 --- a/git.c +++ b/git.c @@ -283,82 +283,25 @@ static int cmd_log(int argc, const char **argv, char **envp) struct rev_info rev; struct commit *commit; char *buf = xmalloc(LOGSIZE); - static enum cmit_fmt commit_format = CMIT_FMT_DEFAULT; - int abbrev = DEFAULT_ABBREV; - 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"); - while (1 < argc) { - const char *arg = argv[1]; - if (!strncmp(arg, "--pretty", 8)) { - commit_format = get_commit_format(arg + 8); - if (commit_format == CMIT_FMT_ONELINE) - commit_prefix = ""; - } - else if (!strcmp(arg, "--no-abbrev")) { - abbrev = 0; - } - else if (!strcmp(arg, "--abbrev")) { - abbrev = DEFAULT_ABBREV; - } - else if (!strcmp(arg, "--abbrev-commit")) { - abbrev_commit = 1; - } - else if (!strncmp(arg, "--abbrev=", 9)) { - abbrev = strtoul(arg + 9, NULL, 10); - if (abbrev && abbrev < MINIMUM_ABBREV) - abbrev = MINIMUM_ABBREV; - 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) { - do_diff = 1; - argv += cnt; - argc -= cnt; - continue; - } - die("unrecognized argument: %s", arg); - } + if (argc > 1) + die("unrecognized argument: %s", argv[1]); - 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; - if (!full_diff && rev.prune_data) - diff_tree_setup_paths(rev.prune_data, &opt.diffopt); - diff_setup_done(&opt.diffopt); - } + rev.no_commit_id = 1; + if (rev.commit_format == CMIT_FMT_ONELINE) + commit_prefix = ""; prepare_revision_walk(&rev); setup_pager(); while ((commit = get_revision(&rev)) != NULL) { - if (shown && do_diff && commit_format != CMIT_FMT_ONELINE) + if (shown && rev.diff && rev.commit_format != CMIT_FMT_ONELINE) putchar('\n'); fputs(commit_prefix, stdout); - if (abbrev_commit && abbrev) - fputs(find_unique_abbrev(commit->object.sha1, abbrev), + if (rev.abbrev_commit && rev.abbrev) + fputs(find_unique_abbrev(commit->object.sha1, rev.abbrev), stdout); else fputs(sha1_to_hex(commit->object.sha1), stdout); @@ -381,15 +324,15 @@ static int cmd_log(int argc, const char **argv, char **envp) parents = parents->next) parents->item->object.flags &= ~TMP_MARK; } - if (commit_format == CMIT_FMT_ONELINE) + if (rev.commit_format == CMIT_FMT_ONELINE) putchar(' '); else putchar('\n'); - pretty_print_commit(commit_format, commit, ~0, buf, - LOGSIZE, abbrev); + pretty_print_commit(rev.commit_format, commit, ~0, buf, + LOGSIZE, rev.abbrev); printf("%s\n", buf); - if (do_diff) - log_tree_commit(&opt, commit); + if (rev.diff) + log_tree_commit(&rev, commit); shown = 1; free(commit->buffer); commit->buffer = NULL; diff --git a/log-tree.c b/log-tree.c index 3d404824a1..04a68e0f57 100644 --- a/log-tree.c +++ b/log-tree.c @@ -3,57 +3,7 @@ #include "commit.h" #include "log-tree.h" -void init_log_tree_opt(struct log_tree_opt *opt) -{ - memset(opt, 0, sizeof *opt); - opt->ignore_merges = 1; - opt->header_prefix = ""; - opt->commit_format = CMIT_FMT_RAW; - diff_setup(&opt->diffopt); -} - -int log_tree_opt_parse(struct log_tree_opt *opt, const char **av, int ac) -{ - const char *arg; - int cnt = diff_opt_parse(&opt->diffopt, av, ac); - if (0 < cnt) - return cnt; - arg = *av; - if (!strcmp(arg, "-r")) - opt->diffopt.recursive = 1; - else if (!strcmp(arg, "-t")) { - opt->diffopt.recursive = 1; - opt->diffopt.tree_in_recursive = 1; - } - else if (!strcmp(arg, "-m")) - opt->ignore_merges = 0; - else if (!strcmp(arg, "-c")) - opt->combine_merges = 1; - else if (!strcmp(arg, "--cc")) { - opt->dense_combined_merges = 1; - opt->combine_merges = 1; - } - else if (!strcmp(arg, "-v")) { - opt->verbose_header = 1; - opt->header_prefix = "diff-tree "; - } - else if (!strncmp(arg, "--pretty", 8)) { - opt->verbose_header = 1; - opt->header_prefix = "diff-tree "; - opt->commit_format = get_commit_format(arg+8); - } - else if (!strcmp(arg, "--root")) - opt->show_root_diff = 1; - else if (!strcmp(arg, "--no-commit-id")) - opt->no_commit_id = 1; - else if (!strcmp(arg, "--always")) - opt->always_show_header = 1; - else - return 0; - return 1; -} - -int log_tree_diff_flush(struct log_tree_opt *opt) +int log_tree_diff_flush(struct rev_info *opt) { diffcore_std(&opt->diffopt); if (diff_queue_is_empty()) { @@ -73,7 +23,7 @@ int log_tree_diff_flush(struct log_tree_opt *opt) return 1; } -static int diff_root_tree(struct log_tree_opt *opt, +static int diff_root_tree(struct rev_info *opt, const unsigned char *new, const char *base) { int retval; @@ -93,7 +43,7 @@ static int diff_root_tree(struct log_tree_opt *opt, return retval; } -static const char *generate_header(struct log_tree_opt *opt, +static const char *generate_header(struct rev_info *opt, const unsigned char *commit_sha1, const unsigned char *parent_sha1, const struct commit *commit) @@ -129,7 +79,7 @@ static const char *generate_header(struct log_tree_opt *opt, return this_header; } -static int do_diff_combined(struct log_tree_opt *opt, struct commit *commit) +static int do_diff_combined(struct rev_info *opt, struct commit *commit) { unsigned const char *sha1 = commit->object.sha1; @@ -142,7 +92,7 @@ static int do_diff_combined(struct log_tree_opt *opt, struct commit *commit) return 0; } -int log_tree_commit(struct log_tree_opt *opt, struct commit *commit) +int log_tree_commit(struct rev_info *opt, struct commit *commit) { struct commit_list *parents; unsigned const char *sha1 = commit->object.sha1; diff --git a/log-tree.h b/log-tree.h index da166c6f2c..91a909be73 100644 --- a/log-tree.h +++ b/log-tree.h @@ -1,23 +1,11 @@ #ifndef LOG_TREE_H #define LOG_TREE_H -struct log_tree_opt { - struct diff_options diffopt; - int show_root_diff; - int no_commit_id; - int verbose_header; - int ignore_merges; - int combine_merges; - int dense_combined_merges; - int always_show_header; - const char *header_prefix; - const char *header; - enum cmit_fmt commit_format; -}; +#include "revision.h" -void init_log_tree_opt(struct log_tree_opt *); -int log_tree_diff_flush(struct log_tree_opt *); -int log_tree_commit(struct log_tree_opt *, struct commit *); -int log_tree_opt_parse(struct log_tree_opt *, const char **, int); +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); #endif diff --git a/revision.c b/revision.c index 0505f3f455..1d26e0d911 100644 --- a/revision.c +++ b/revision.c @@ -116,21 +116,27 @@ static void add_pending_object(struct rev_info *revs, struct object *obj, const add_object(obj, &revs->pending_objects, NULL, name); } -static struct commit *get_commit_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags) +static struct object *get_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags) { struct object *object; object = parse_object(sha1); if (!object) die("bad object %s", name); + object->flags |= flags; + return object; +} + +static struct commit *handle_commit(struct rev_info *revs, struct object *object, const char *name) +{ + unsigned long flags = object->flags; /* * Tag object? Look what it points to.. */ while (object->type == tag_type) { struct tag *tag = (struct tag *) object; - object->flags |= flags; - if (revs->tag_objects && !(object->flags & UNINTERESTING)) + if (revs->tag_objects && !(flags & UNINTERESTING)) add_pending_object(revs, object, tag->tag); object = parse_object(tag->tagged->sha1); if (!object) @@ -143,7 +149,6 @@ static struct commit *get_commit_reference(struct rev_info *revs, const char *na */ if (object->type == commit_type) { struct commit *commit = (struct commit *)object; - object->flags |= flags; if (parse_commit(commit) < 0) die("unable to parse commit %s", name); if (flags & UNINTERESTING) { @@ -241,7 +246,7 @@ int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2) return REV_TREE_DIFFERENT; tree_difference = REV_TREE_SAME; if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "", - &revs->diffopt) < 0) + &revs->pruning) < 0) return REV_TREE_DIFFERENT; return tree_difference; } @@ -264,7 +269,7 @@ int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1) empty.size = 0; tree_difference = 0; - retval = diff_tree(&empty, &real, "", &revs->diffopt); + retval = diff_tree(&empty, &real, "", &revs->pruning); free(tree); return retval >= 0 && !tree_difference; @@ -451,21 +456,13 @@ static void limit_list(struct rev_info *revs) revs->commits = newlist; } -static void add_one_commit(struct commit *commit, struct rev_info *revs) -{ - if (!commit || (commit->object.flags & SEEN)) - return; - commit->object.flags |= SEEN; - commit_list_insert(commit, &revs->commits); -} - static int all_flags; static struct rev_info *all_revs; static int handle_one_ref(const char *path, const unsigned char *sha1) { - struct commit *commit = get_commit_reference(all_revs, path, sha1, all_flags); - add_one_commit(commit, all_revs); + struct object *object = get_reference(all_revs, path, sha1, all_flags); + add_pending_object(all_revs, object, ""); return 0; } @@ -479,9 +476,9 @@ static void handle_all(struct rev_info *revs, unsigned flags) void init_revisions(struct rev_info *revs) { memset(revs, 0, sizeof(*revs)); - revs->diffopt.recursive = 1; - revs->diffopt.add_remove = file_add_remove; - revs->diffopt.change = file_change; + revs->pruning.recursive = 1; + revs->pruning.add_remove = file_add_remove; + revs->pruning.change = file_change; revs->lifo = 1; revs->dense = 1; revs->prefix = setup_git_directory(); @@ -494,6 +491,11 @@ 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); } /* @@ -526,13 +528,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch flags = 0; for (i = 1; i < argc; i++) { - struct commit *commit; + struct object *object; const char *arg = argv[i]; unsigned char sha1[20]; char *dotdot; int local_flags; if (*arg == '-') { + int opts; if (!strncmp(arg, "--max-count=", 12)) { revs->max_count = atoi(arg + 12); continue; @@ -640,6 +643,78 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch revs->unpacked = 1; continue; } + if (!strcmp(arg, "-r")) { + revs->diff = 1; + revs->diffopt.recursive = 1; + continue; + } + if (!strcmp(arg, "-t")) { + revs->diff = 1; + revs->diffopt.recursive = 1; + revs->diffopt.tree_in_recursive = 1; + continue; + } + if (!strcmp(arg, "-m")) { + revs->ignore_merges = 0; + continue; + } + if (!strcmp(arg, "-c")) { + revs->diff = 1; + revs->combine_merges = 1; + continue; + } + if (!strcmp(arg, "--cc")) { + revs->diff = 1; + revs->dense_combined_merges = 1; + revs->combine_merges = 1; + continue; + } + 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; + } + if (!strcmp(arg, "--root")) { + revs->show_root_diff = 1; + continue; + } + if (!strcmp(arg, "--no-commit-id")) { + revs->no_commit_id = 1; + continue; + } + if (!strcmp(arg, "--always")) { + revs->always_show_header = 1; + continue; + } + if (!strcmp(arg, "--no-abbrev")) { + revs->abbrev = 0; + continue; + } + if (!strcmp(arg, "--abbrev")) { + revs->abbrev = DEFAULT_ABBREV; + continue; + } + if (!strcmp(arg, "--abbrev-commit")) { + revs->abbrev_commit = 1; + continue; + } + if (!strcmp(arg, "--full-diff")) { + revs->diff = 1; + revs->full_diff = 1; + continue; + } + opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i); + if (opts > 0) { + revs->diff = 1; + i += opts - 1; + continue; + } *unrecognized++ = arg; left++; continue; @@ -656,15 +731,15 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch this = "HEAD"; if (!get_sha1(this, from_sha1) && !get_sha1(next, sha1)) { - struct commit *exclude; - struct commit *include; + struct object *exclude; + struct object *include; - exclude = get_commit_reference(revs, this, from_sha1, flags ^ UNINTERESTING); - include = get_commit_reference(revs, next, sha1, flags); + exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING); + include = get_reference(revs, next, sha1, flags); if (!exclude || !include) die("Invalid revision range %s..%s", arg, next); - add_one_commit(exclude, revs); - add_one_commit(include, revs); + add_pending_object(revs, exclude, this); + add_pending_object(revs, include, next); continue; } *dotdot = '.'; @@ -689,32 +764,57 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch revs->prune_data = get_pathspec(revs->prefix, argv + i); break; } - commit = get_commit_reference(revs, arg, sha1, flags ^ local_flags); - add_one_commit(commit, revs); + object = get_reference(revs, arg, sha1, flags ^ local_flags); + add_pending_object(revs, object, arg); } - if (def && !revs->commits) { + if (def && !revs->pending_objects) { unsigned char sha1[20]; - struct commit *commit; + struct object *object; if (get_sha1(def, sha1) < 0) die("bad default revision '%s'", def); - commit = get_commit_reference(revs, def, sha1, 0); - add_one_commit(commit, revs); + object = get_reference(revs, def, sha1, 0); + add_pending_object(revs, object, def); } if (revs->topo_order || revs->unpacked) revs->limited = 1; if (revs->prune_data) { - diff_tree_setup_paths(revs->prune_data, &revs->diffopt); + diff_tree_setup_paths(revs->prune_data, &revs->pruning); revs->prune_fn = try_to_simplify_commit; + if (!revs->full_diff) + diff_tree_setup_paths(revs->prune_data, &revs->diffopt); } + if (revs->combine_merges) { + revs->ignore_merges = 0; + if (revs->dense_combined_merges) + revs->diffopt.output_format = DIFF_FORMAT_PATCH; + } + if (revs->diffopt.output_format == DIFF_FORMAT_PATCH) + revs->diffopt.recursive = 1; + revs->diffopt.abbrev = revs->abbrev; + diff_setup_done(&revs->diffopt); return left; } void prepare_revision_walk(struct rev_info *revs) { - sort_by_date(&revs->commits); + struct object_list *list; + + list = revs->pending_objects; + revs->pending_objects = NULL; + while (list) { + struct commit *commit = handle_commit(revs, list->item, list->name); + if (commit) { + if (!(commit->object.flags & SEEN)) { + commit->object.flags |= SEEN; + insert_by_date(commit, &revs->commits); + } + } + list = list->next; + } + if (revs->limited) limit_list(revs); if (revs->topo_order) diff --git a/revision.h b/revision.h index 8970b57e3c..6eaa9048a9 100644 --- a/revision.h +++ b/revision.h @@ -38,13 +38,32 @@ struct rev_info { boundary:1, parents:1; + /* Diff flags */ + unsigned int diff:1, + full_diff:1, + show_root_diff:1, + no_commit_id:1, + verbose_header:1, + ignore_merges:1, + combine_merges:1, + dense_combined_merges:1, + always_show_header:1; + + /* Format info */ + unsigned int abbrev_commit:1; + unsigned int abbrev; + enum cmit_fmt commit_format; + const char *header_prefix; + const char *header; + /* special limits */ int max_count; unsigned long max_age; unsigned long min_age; - /* paths limiting */ + /* diff info for patches and for paths limiting */ struct diff_options diffopt; + struct diff_options pruning; topo_sort_set_fn_t topo_setter; topo_sort_get_fn_t topo_getter; From 8e8f998739db6526fe890fabc88c866759bc2ac3 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 14 Apr 2006 22:19:38 -0700 Subject: [PATCH 02/17] Fix up default abbrev in setup_revisions() argument parser. The default abbreviation precision should be DEFAULT_ABBREV as before. Signed-off-by: Junio C Hamano --- diff-tree.c | 1 + git.c | 1 + revision.c | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/diff-tree.c b/diff-tree.c index 54157e40dd..979f792b67 100644 --- a/diff-tree.c +++ b/diff-tree.c @@ -69,6 +69,7 @@ int main(int argc, const char **argv) git_config(git_diff_config); nr_sha1 = 0; + opt->abbrev = 0; argc = setup_revisions(argc, argv, opt, NULL); while (--argc > 0) { diff --git a/git.c b/git.c index 437e9b57d0..9d885569d8 100644 --- a/git.c +++ b/git.c @@ -286,6 +286,7 @@ static int cmd_log(int argc, const char **argv, char **envp) const char *commit_prefix = "commit "; int shown = 0; + rev.abbrev = DEFAULT_ABBREV; argc = setup_revisions(argc, argv, &rev, "HEAD"); if (argc > 1) die("unrecognized argument: %s", argv[1]); diff --git a/revision.c b/revision.c index 1d26e0d911..bdf8005aec 100644 --- a/revision.c +++ b/revision.c @@ -475,7 +475,12 @@ static void handle_all(struct rev_info *revs, unsigned flags) void init_revisions(struct rev_info *revs) { + unsigned abbrev = revs->abbrev; + memset(revs, 0, sizeof(*revs)); + + revs->abbrev = abbrev; + revs->ignore_merges = 1; revs->pruning.recursive = 1; revs->pruning.add_remove = file_add_remove; revs->pruning.change = file_change; From 8c1f0b44c59530dea8007a9f5b69d0fac6aea3b1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 14 Apr 2006 22:43:34 -0700 Subject: [PATCH 03/17] Fix up rev-list option parsing. rev-list does not take diff options, so barf after seeing some. Signed-off-by: Junio C Hamano --- rev-list.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rev-list.c b/rev-list.c index 963707a495..0de21810c9 100644 --- a/rev-list.c +++ b/rev-list.c @@ -365,8 +365,10 @@ int main(int argc, const char **argv) list = revs.commits; - if (!list && - (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) && !revs.pending_objects)) + if ((!list && + (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) && + !revs.pending_objects)) || + revs.diff) usage(rev_list_usage); save_commit_buffer = verbose_header; From 6b9c58f4669b3832ed2830f0cb1a307ea6bc6063 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 15 Apr 2006 23:46:36 -0700 Subject: [PATCH 04/17] Split init_revisions() out of setup_revisions() Merging all three option parsers related to whatchanged is unarguably the right thing, but the fallout was too big to scare me away. Let's try it once again, but once step at time. This splits out init_revisions() call from setup_revisions(), so that the callers can set different defaults to match the traditional benaviour. The rev-list command is still broken in a big way, which is the topic of next step. Signed-off-by: Junio C Hamano --- commit.h | 2 ++ diff-tree.c | 1 + git.c | 1 + http-push.c | 1 + revision.c | 6 +----- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/commit.h b/commit.h index 918c9ab5e4..de142afe73 100644 --- a/commit.h +++ b/commit.h @@ -45,6 +45,8 @@ enum cmit_fmt { CMIT_FMT_FULL, CMIT_FMT_FULLER, CMIT_FMT_ONELINE, + + CMIT_FMT_UNSPECIFIED, }; extern enum cmit_fmt get_commit_format(const char *arg); diff --git a/diff-tree.c b/diff-tree.c index 979f792b67..e578798537 100644 --- a/diff-tree.c +++ b/diff-tree.c @@ -69,6 +69,7 @@ int main(int argc, const char **argv) git_config(git_diff_config); nr_sha1 = 0; + init_revisions(opt); opt->abbrev = 0; argc = setup_revisions(argc, argv, opt, NULL); diff --git a/git.c b/git.c index 9d885569d8..d172626230 100644 --- a/git.c +++ b/git.c @@ -286,6 +286,7 @@ static int cmd_log(int argc, const char **argv, char **envp) const char *commit_prefix = "commit "; int shown = 0; + init_revisions(&rev); rev.abbrev = DEFAULT_ABBREV; argc = setup_revisions(argc, argv, &rev, "HEAD"); if (argc > 1) diff --git a/http-push.c b/http-push.c index 19a0f772e7..4a9dcf2bf6 100644 --- a/http-push.c +++ b/http-push.c @@ -2498,6 +2498,7 @@ int main(int argc, char **argv) commit_argv[3] = old_sha1_hex; commit_argc++; } + init_revisions(&revs); setup_revisions(commit_argc, commit_argv, &revs, NULL); free(new_sha1_hex); if (old_sha1_hex) { diff --git a/revision.c b/revision.c index bdf8005aec..9693b6e4c7 100644 --- a/revision.c +++ b/revision.c @@ -475,11 +475,9 @@ static void handle_all(struct rev_info *revs, unsigned flags) void init_revisions(struct rev_info *revs) { - unsigned abbrev = revs->abbrev; - memset(revs, 0, sizeof(*revs)); - revs->abbrev = abbrev; + revs->abbrev = DEFAULT_ABBREV; revs->ignore_merges = 1; revs->pruning.recursive = 1; revs->pruning.add_remove = file_add_remove; @@ -516,8 +514,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch const char **unrecognized = argv + 1; int left = 1; - init_revisions(revs); - /* First, search for "--" */ seen_dashdash = 0; for (i = 1; i < argc; i++) { From 7594c4b2d7cc81f806453402aefe1bf71ae6dd53 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 15 Apr 2006 23:48:27 -0700 Subject: [PATCH 05/17] rev-list option parser fix. The big option parser unification broke rev-list the big way; this makes it use options from the parsed revs structure. Signed-off-by: Junio C Hamano --- rev-list.c | 82 +++++++++++++++--------------------------------------- 1 file changed, 23 insertions(+), 59 deletions(-) diff --git a/rev-list.c b/rev-list.c index 0de21810c9..000f27a8fe 100644 --- a/rev-list.c +++ b/rev-list.c @@ -39,24 +39,20 @@ static const char rev_list_usage[] = struct rev_info revs; static int bisect_list = 0; -static int verbose_header = 0; -static int abbrev = DEFAULT_ABBREV; -static int abbrev_commit = 0; static int show_timestamp = 0; static int hdr_termination = 0; -static const char *commit_prefix = ""; -static enum cmit_fmt commit_format = CMIT_FMT_RAW; static void show_commit(struct commit *commit) { if (show_timestamp) printf("%lu ", commit->date); - if (commit_prefix[0]) - fputs(commit_prefix, stdout); + if (*revs.header_prefix) + fputs(revs.header_prefix, stdout); if (commit->object.flags & BOUNDARY) putchar('-'); - if (abbrev_commit && abbrev) - fputs(find_unique_abbrev(commit->object.sha1, abbrev), stdout); + if (revs.abbrev_commit && revs.abbrev) + fputs(find_unique_abbrev(commit->object.sha1, revs.abbrev), + stdout); else fputs(sha1_to_hex(commit->object.sha1), stdout); if (revs.parents) { @@ -78,14 +74,16 @@ static void show_commit(struct commit *commit) parents = parents->next) parents->item->object.flags &= ~TMP_MARK; } - if (commit_format == CMIT_FMT_ONELINE) + if (revs.commit_format == CMIT_FMT_ONELINE) putchar(' '); else putchar('\n'); - if (verbose_header) { + if (revs.verbose_header) { static char pretty_header[16384]; - pretty_print_commit(commit_format, commit, ~0, pretty_header, sizeof(pretty_header), abbrev); + pretty_print_commit(revs.commit_format, commit, ~0, + pretty_header, sizeof(pretty_header), + revs.abbrev); printf("%s%c", pretty_header, hdr_termination); } fflush(stdout); @@ -297,58 +295,16 @@ int main(int argc, const char **argv) struct commit_list *list; int i; + init_revisions(&revs); + revs.abbrev = 0; + revs.commit_format = CMIT_FMT_UNSPECIFIED; argc = setup_revisions(argc, argv, &revs, NULL); for (i = 1 ; i < argc; i++) { const char *arg = argv[i]; - /* accept -, like traditilnal "head" */ - if ((*arg == '-') && isdigit(arg[1])) { - revs.max_count = atoi(arg + 1); - continue; - } - if (!strcmp(arg, "-n")) { - if (++i >= argc) - die("-n requires an argument"); - revs.max_count = atoi(argv[i]); - continue; - } - if (!strncmp(arg,"-n",2)) { - revs.max_count = atoi(arg + 2); - continue; - } if (!strcmp(arg, "--header")) { - verbose_header = 1; - continue; - } - if (!strcmp(arg, "--no-abbrev")) { - abbrev = 0; - continue; - } - if (!strcmp(arg, "--abbrev")) { - abbrev = DEFAULT_ABBREV; - continue; - } - if (!strcmp(arg, "--abbrev-commit")) { - abbrev_commit = 1; - continue; - } - if (!strncmp(arg, "--abbrev=", 9)) { - abbrev = strtoul(arg + 9, NULL, 10); - if (abbrev && abbrev < MINIMUM_ABBREV) - abbrev = MINIMUM_ABBREV; - else if (40 < abbrev) - abbrev = 40; - continue; - } - if (!strncmp(arg, "--pretty", 8)) { - commit_format = get_commit_format(arg+8); - verbose_header = 1; - hdr_termination = '\n'; - if (commit_format == CMIT_FMT_ONELINE) - commit_prefix = ""; - else - commit_prefix = "commit "; + revs.verbose_header = 1; continue; } if (!strcmp(arg, "--timestamp")) { @@ -362,6 +318,14 @@ int main(int argc, const char **argv) usage(rev_list_usage); } + if (revs.commit_format != CMIT_FMT_UNSPECIFIED) { + /* The command line has a --pretty */ + hdr_termination = '\n'; + if (revs.commit_format == CMIT_FMT_ONELINE) + revs.header_prefix = ""; + else + revs.header_prefix = "commit "; + } list = revs.commits; @@ -371,7 +335,7 @@ int main(int argc, const char **argv) revs.diff) usage(rev_list_usage); - save_commit_buffer = verbose_header; + save_commit_buffer = revs.verbose_header; track_object_refs = 0; prepare_revision_walk(&revs); From 5b84f4d87a1bd58c7540e9ea82ee3673ecddbad5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 16 Apr 2006 00:07:41 -0700 Subject: [PATCH 06/17] Built-in git-whatchanged. Split internal "git log" into reusable piece and add "git whatchanged". This is based on the option parsing unification work Linus did. Signed-off-by: Junio C Hamano --- git.c | 62 +++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/git.c b/git.c index d172626230..9a89b0afbd 100644 --- a/git.c +++ b/git.c @@ -278,36 +278,33 @@ static int cmd_help(int argc, const char **argv, char **envp) #define LOGSIZE (65536) -static int cmd_log(int argc, const char **argv, char **envp) +static int cmd_log_wc(int argc, const char **argv, char **envp, + struct rev_info *rev) { - struct rev_info rev; struct commit *commit; char *buf = xmalloc(LOGSIZE); const char *commit_prefix = "commit "; int shown = 0; - init_revisions(&rev); - rev.abbrev = DEFAULT_ABBREV; - argc = setup_revisions(argc, argv, &rev, "HEAD"); if (argc > 1) die("unrecognized argument: %s", argv[1]); - - rev.no_commit_id = 1; - if (rev.commit_format == CMIT_FMT_ONELINE) + if (rev->commit_format == CMIT_FMT_ONELINE) commit_prefix = ""; - prepare_revision_walk(&rev); + prepare_revision_walk(rev); setup_pager(); - while ((commit = get_revision(&rev)) != NULL) { - if (shown && rev.diff && rev.commit_format != CMIT_FMT_ONELINE) + while ((commit = get_revision(rev)) != NULL) { + if (shown && rev->diff && + rev->commit_format != CMIT_FMT_ONELINE) putchar('\n'); fputs(commit_prefix, stdout); - if (rev.abbrev_commit && rev.abbrev) - fputs(find_unique_abbrev(commit->object.sha1, rev.abbrev), + if (rev->abbrev_commit && rev->abbrev) + fputs(find_unique_abbrev(commit->object.sha1, + rev->abbrev), stdout); else fputs(sha1_to_hex(commit->object.sha1), stdout); - if (rev.parents) { + if (rev->parents) { struct commit_list *parents = commit->parents; while (parents) { struct object *o = &(parents->item->object); @@ -326,15 +323,15 @@ static int cmd_log(int argc, const char **argv, char **envp) parents = parents->next) parents->item->object.flags &= ~TMP_MARK; } - if (rev.commit_format == CMIT_FMT_ONELINE) + if (rev->commit_format == CMIT_FMT_ONELINE) putchar(' '); else putchar('\n'); - pretty_print_commit(rev.commit_format, commit, ~0, buf, - LOGSIZE, rev.abbrev); + pretty_print_commit(rev->commit_format, commit, ~0, buf, + LOGSIZE, rev->abbrev); printf("%s\n", buf); - if (rev.diff) - log_tree_commit(&rev, commit); + if (rev->diff) + log_tree_commit(rev, commit); shown = 1; free(commit->buffer); commit->buffer = NULL; @@ -343,6 +340,32 @@ static int cmd_log(int argc, const char **argv, char **envp) return 0; } +static int cmd_wc(int argc, const char **argv, char **envp) +{ + struct rev_info rev; + + init_revisions(&rev); + rev.abbrev = DEFAULT_ABBREV; + rev.no_commit_id = 1; + rev.commit_format = CMIT_FMT_DEFAULT; + rev.diff = 1; + rev.diffopt.recursive = 1; + argc = setup_revisions(argc, argv, &rev, "HEAD"); + return cmd_log_wc(argc, argv, envp, &rev); +} + +static int cmd_log(int argc, const char **argv, char **envp) +{ + struct rev_info rev; + + init_revisions(&rev); + rev.abbrev = DEFAULT_ABBREV; + rev.no_commit_id = 1; + rev.commit_format = CMIT_FMT_DEFAULT; + argc = setup_revisions(argc, argv, &rev, "HEAD"); + return cmd_log_wc(argc, argv, envp, &rev); +} + static void handle_internal_command(int argc, const char **argv, char **envp) { const char *cmd = argv[0]; @@ -353,6 +376,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp) { "version", cmd_version }, { "help", cmd_help }, { "log", cmd_log }, + { "whatchanged", cmd_wc }, }; int i; From ba1d45051e050cbcf68ccccacea86a4b6ecde731 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 15 Apr 2006 12:09:56 -0700 Subject: [PATCH 07/17] Tentative built-in "git show" This uses the "--no-walk" flag that I never actually implemented (but I'm sure I mentioned it) to make "git show" be essentially the same thing as "git whatchanged --no-walk". It just refuses to add more interesting parents to the revision walking history, so you don't actually get any history, you just get the commit you asked for. I was going to add "--no-walk" as a real argument flag to git-rev-list too, but I'm not sure anybody actually needs it. Although it might be useful for porcelain, so I left the door open. [jc: ported to the unified option structure by Linus] Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- git.c | 18 ++++++++++++++++++ revision.c | 5 +++++ revision.h | 1 + 3 files changed, 24 insertions(+) diff --git a/git.c b/git.c index 9a89b0afbd..9e29ade2b4 100644 --- a/git.c +++ b/git.c @@ -354,6 +354,23 @@ static int cmd_wc(int argc, const char **argv, char **envp) return cmd_log_wc(argc, argv, envp, &rev); } +static int cmd_show(int argc, const char **argv, char **envp) +{ + struct rev_info rev; + + init_revisions(&rev); + rev.diff = 1; + rev.ignore_merges = 0; + rev.combine_merges = 1; + rev.dense_combined_merges = 1; + rev.abbrev = DEFAULT_ABBREV; + rev.commit_format = CMIT_FMT_DEFAULT; + rev.diffopt.recursive = 1; + rev.no_walk = 1; + argc = setup_revisions(argc, argv, &rev, "HEAD"); + return cmd_log_wc(argc, argv, envp, &rev); +} + static int cmd_log(int argc, const char **argv, char **envp) { struct rev_info rev; @@ -377,6 +394,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp) { "help", cmd_help }, { "log", cmd_log }, { "whatchanged", cmd_wc }, + { "show", cmd_show }, }; int i; diff --git a/revision.c b/revision.c index 9693b6e4c7..f8fb028855 100644 --- a/revision.c +++ b/revision.c @@ -380,6 +380,9 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st if (revs->prune_fn) revs->prune_fn(revs, commit); + if (revs->no_walk) + return; + parent = commit->parents; while (parent) { struct commit *p = parent->item; @@ -816,6 +819,8 @@ void prepare_revision_walk(struct rev_info *revs) list = list->next; } + if (revs->no_walk) + return; if (revs->limited) limit_list(revs); if (revs->topo_order) diff --git a/revision.h b/revision.h index 6eaa9048a9..7b854866b2 100644 --- a/revision.h +++ b/revision.h @@ -26,6 +26,7 @@ struct rev_info { /* Traversal flags */ unsigned int dense:1, no_merges:1, + no_walk:1, remove_empty_trees:1, lifo:1, topo_order:1, From d4ed9793fd981ea5a35ebaa8e337446bb29f6d55 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 16 Apr 2006 02:42:00 -0700 Subject: [PATCH 08/17] Simplify common default options setup for built-in log family. Signed-off-by: Junio C Hamano --- git.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/git.c b/git.c index a6cfd1d8aa..cc43bfcaba 100644 --- a/git.c +++ b/git.c @@ -286,6 +286,11 @@ static int cmd_log_wc(int argc, const char **argv, char **envp, const char *commit_prefix = "commit "; int shown = 0; + rev->abbrev = DEFAULT_ABBREV; + rev->commit_format = CMIT_FMT_DEFAULT; + rev->no_commit_id = 1; + argc = setup_revisions(argc, argv, rev, "HEAD"); + if (argc > 1) die("unrecognized argument: %s", argv[1]); if (rev->commit_format == CMIT_FMT_ONELINE) @@ -347,12 +352,8 @@ static int cmd_wc(int argc, const char **argv, char **envp) struct rev_info rev; init_revisions(&rev); - rev.abbrev = DEFAULT_ABBREV; - rev.no_commit_id = 1; - rev.commit_format = CMIT_FMT_DEFAULT; rev.diff = 1; rev.diffopt.recursive = 1; - argc = setup_revisions(argc, argv, &rev, "HEAD"); return cmd_log_wc(argc, argv, envp, &rev); } @@ -362,14 +363,11 @@ static int cmd_show(int argc, const char **argv, char **envp) init_revisions(&rev); rev.diff = 1; - rev.ignore_merges = 0; + rev.diffopt.recursive = 1; rev.combine_merges = 1; rev.dense_combined_merges = 1; - rev.abbrev = DEFAULT_ABBREV; - rev.commit_format = CMIT_FMT_DEFAULT; - rev.diffopt.recursive = 1; + rev.ignore_merges = 0; rev.no_walk = 1; - argc = setup_revisions(argc, argv, &rev, "HEAD"); return cmd_log_wc(argc, argv, envp, &rev); } @@ -378,10 +376,6 @@ static int cmd_log(int argc, const char **argv, char **envp) struct rev_info rev; init_revisions(&rev); - rev.abbrev = DEFAULT_ABBREV; - rev.no_commit_id = 1; - rev.commit_format = CMIT_FMT_DEFAULT; - argc = setup_revisions(argc, argv, &rev, "HEAD"); return cmd_log_wc(argc, argv, envp, &rev); } From cb8f64b4e3f263c113b7a2f156af74b810e969ff Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 16 Apr 2006 03:29:10 -0700 Subject: [PATCH 09/17] log/whatchanged/show - log formatting cleanup. This moves the decision to print the log message, while diff options are in effect, to log-tree. It gives behaviour closer to the traditional one. Signed-off-by: Junio C Hamano --- git.c | 41 ++++++++++++++++++++++++++--------------- log-tree.c | 3 +++ revision.h | 1 + 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/git.c b/git.c index cc43bfcaba..c5de8d3a12 100644 --- a/git.c +++ b/git.c @@ -288,7 +288,6 @@ static int cmd_log_wc(int argc, const char **argv, char **envp, rev->abbrev = DEFAULT_ABBREV; rev->commit_format = CMIT_FMT_DEFAULT; - rev->no_commit_id = 1; argc = setup_revisions(argc, argv, rev, "HEAD"); if (argc > 1) @@ -299,16 +298,20 @@ static int cmd_log_wc(int argc, const char **argv, char **envp, 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'); - fputs(commit_prefix, stdout); + + ofs = sprintf(buf, "%s", commit_prefix); if (rev->abbrev_commit && rev->abbrev) - fputs(find_unique_abbrev(commit->object.sha1, - rev->abbrev), - stdout); + ofs += sprintf(buf + ofs, "%s", + find_unique_abbrev(commit->object.sha1, + rev->abbrev)); else - fputs(sha1_to_hex(commit->object.sha1), stdout); + ofs += sprintf(buf + ofs, "%s", + sha1_to_hex(commit->object.sha1)); if (rev->parents) { struct commit_list *parents = commit->parents; while (parents) { @@ -316,7 +319,8 @@ static int cmd_log_wc(int argc, const char **argv, char **envp, parents = parents->next; if (o->flags & TMP_MARK) continue; - printf(" %s", sha1_to_hex(o->sha1)); + ofs += sprintf(buf + ofs, " %s", + sha1_to_hex(o->sha1)); o->flags |= TMP_MARK; } /* TMP_MARK is a general purpose flag that can @@ -328,17 +332,20 @@ static int cmd_log_wc(int argc, const char **argv, char **envp, parents = parents->next) parents->item->object.flags &= ~TMP_MARK; } - if (rev->commit_format == CMIT_FMT_ONELINE) - putchar(' '); - else - putchar('\n'); - pretty_print_commit(rev->commit_format, commit, ~0, buf, - LOGSIZE, rev->abbrev); - printf("%s\n", buf); + 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) { - printf("---\n"); + rev->use_precomputed_header = buf; + strcpy(buf + ofs, "\n---\n"); log_tree_commit(rev, commit); } + else + printf("%s\n", buf); shown = 1; free(commit->buffer); commit->buffer = NULL; @@ -376,6 +383,10 @@ static int cmd_log(int argc, const char **argv, char **envp) struct rev_info rev; init_revisions(&rev); + rev.always_show_header = 1; + rev.diffopt.recursive = 1; + rev.combine_merges = 1; + rev.ignore_merges = 0; return cmd_log_wc(argc, argv, envp, &rev); } diff --git a/log-tree.c b/log-tree.c index 04a68e0f57..7d9f41ede1 100644 --- a/log-tree.c +++ b/log-tree.c @@ -54,6 +54,9 @@ static const char *generate_header(struct rev_info *opt, 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); diff --git a/revision.h b/revision.h index 7b854866b2..74dc5ef179 100644 --- a/revision.h +++ b/revision.h @@ -56,6 +56,7 @@ struct rev_info { enum cmit_fmt commit_format; const char *header_prefix; const char *header; + const char *use_precomputed_header; /* special limits */ int max_count; From 78fff6ebbafe2d23464a2b059104954bfe8732c7 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sun, 16 Apr 2006 15:17:23 -0700 Subject: [PATCH 10/17] Fixes for option parsing Make sure "git show" always show the header, regardless of whether there is a diff or not. Also, make sure "always_show_header" actually works, since generate_header only tested it in one out of three return paths. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- git.c | 1 + log-tree.c | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/git.c b/git.c index c5de8d3a12..fc4e429278 100644 --- a/git.c +++ b/git.c @@ -373,6 +373,7 @@ static int cmd_show(int argc, const char **argv, char **envp) rev.diffopt.recursive = 1; rev.combine_merges = 1; rev.dense_combined_merges = 1; + rev.always_show_header = 1; rev.ignore_merges = 0; rev.no_walk = 1; return cmd_log_wc(argc, argv, envp, &rev); diff --git a/log-tree.c b/log-tree.c index 7d9f41ede1..af36f702e1 100644 --- a/log-tree.c +++ b/log-tree.c @@ -43,7 +43,7 @@ static int diff_root_tree(struct rev_info *opt, return retval; } -static const char *generate_header(struct rev_info *opt, +static const char *get_header(struct rev_info *opt, const unsigned char *commit_sha1, const unsigned char *parent_sha1, const struct commit *commit) @@ -75,13 +75,23 @@ static const char *generate_header(struct rev_info *opt, offset += pretty_print_commit(opt->commit_format, commit, len, this_header + offset, sizeof(this_header) - offset, abbrev); - if (opt->always_show_header) { - puts(this_header); - return NULL; - } 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; From db89665fbf86d4e0166b2f252a939ed8bf6782fe Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Apr 2006 12:42:36 -0700 Subject: [PATCH 11/17] 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 12/17] 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 13/17] 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 14/17] 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 15/17] 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 16/17] 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 */ From 3a624b346db02a07b0317743b362d1a15c6c3c18 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 18 Apr 2006 11:41:28 -0700 Subject: [PATCH 17/17] Fix "git log --stat": make sure to set recursive with --stat. Just like "patch" format always needs recursive, "diffstat" format does not make sense without setting recursive. Signed-off-by: Junio C Hamano --- diff.c | 9 +++++++++ revision.c | 2 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/diff.c b/diff.c index b54bbfa627..d75e0178ff 100644 --- a/diff.c +++ b/diff.c @@ -1029,6 +1029,15 @@ int diff_setup_done(struct diff_options *options) options->detect_rename != DIFF_DETECT_COPY) || (0 <= options->rename_limit && !options->detect_rename)) return -1; + + /* + * These cases always need recursive; we do not drop caller-supplied + * recursive bits for other formats here. + */ + if ((options->output_format == DIFF_FORMAT_PATCH) || + (options->output_format == DIFF_FORMAT_DIFFSTAT)) + options->recursive = 1; + if (options->detect_rename && options->rename_limit < 0) options->rename_limit = diff_rename_limit_default; if (options->setup & DIFF_SETUP_USE_CACHE) { diff --git a/revision.c b/revision.c index 5abec18bc1..4d2a64ef6b 100644 --- a/revision.c +++ b/revision.c @@ -791,8 +791,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch if (revs->dense_combined_merges) revs->diffopt.output_format = DIFF_FORMAT_PATCH; } - if (revs->diffopt.output_format == DIFF_FORMAT_PATCH) - revs->diffopt.recursive = 1; revs->diffopt.abbrev = revs->abbrev; diff_setup_done(&revs->diffopt);