* [take 2] revision parsing made incremental
@ 2008-07-08 13:19 Pierre Habouzit
2008-07-08 13:19 ` [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions Pierre Habouzit
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin
Following Dscho's remarks, I reworked the series to avoid changing
setup_revisions semantics for now, and only exposed the part that groks
options (and keep pseudo revision arguments out).
It indeed makes the series smaller, even if the first patch is quite
long to read, and is just enough for simplifying git-blame in a very
satisfying way.
The series passes the testsuite, has no know blanks issues, and is
pushed to my public repository.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions. 2008-07-08 13:19 [take 2] revision parsing made incremental Pierre Habouzit @ 2008-07-08 13:19 ` Pierre Habouzit 2008-07-08 13:19 ` [PATCH 2/3] git-blame: migrate to incremental parse-option [1/2] Pierre Habouzit 2008-07-08 17:41 ` [take 2] revision parsing made incremental Linus Torvalds 2008-07-08 22:57 ` Junio C Hamano 2 siblings, 1 reply; 6+ messages in thread From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw) To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit struct rev_info gains two new field: * .def to store --default argument; * .show_merge 1-bit field. handle_revision_opt is able to deal with any revision option, and consumes them, and leaves revision arguments or pseudo arguments (like --all, --not, ...) in place. For now setup_revisions does a pass of handle_revision_opt again so that code not using it in a parse-opt parser still work the same. Signed-off-by: Pierre Habouzit <madcoder@debian•org> --- revision.c | 612 ++++++++++++++++++++++++++++-------------------------------- revision.h | 4 + 2 files changed, 290 insertions(+), 326 deletions(-) diff --git a/revision.c b/revision.c index 0191160..3dbb7a8 100644 --- a/revision.c +++ b/revision.c @@ -974,6 +974,276 @@ static void add_ignore_packed(struct rev_info *revs, const char *name) revs->ignore_packed[num] = NULL; } +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv, + int *unkc, const char **unkv) +{ + const char *arg = argv[0]; + + /* pseudo revision arguments */ + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") || + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") || + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") || + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk")) + { + unkv[(*unkc)++] = arg; + return 0; + } + + if (!prefixcmp(arg, "--max-count=")) { + revs->max_count = atoi(arg + 12); + } + else if (!prefixcmp(arg, "--skip=")) { + revs->skip_count = atoi(arg + 7); + } + /* accept -<digit>, like traditional "head" */ + else if ((*arg == '-') && isdigit(arg[1])) { + revs->max_count = atoi(arg + 1); + } + else if (!strcmp(arg, "-n")) { + if (argc <= 1) + return error("-n requires an argument"); + revs->max_count = atoi(argv[1]); + return 2; + } + else if (!prefixcmp(arg, "-n")) { + revs->max_count = atoi(arg + 2); + } + else if (!prefixcmp(arg, "--max-age=")) { + revs->max_age = atoi(arg + 10); + } + else if (!prefixcmp(arg, "--since=")) { + revs->max_age = approxidate(arg + 8); + } + else if (!prefixcmp(arg, "--after=")) { + revs->max_age = approxidate(arg + 8); + } + else if (!prefixcmp(arg, "--min-age=")) { + revs->min_age = atoi(arg + 10); + } + else if (!prefixcmp(arg, "--before=")) { + revs->min_age = approxidate(arg + 9); + } + else if (!prefixcmp(arg, "--until=")) { + revs->min_age = approxidate(arg + 8); + } + else if (!strcmp(arg, "--first-parent")) { + revs->first_parent_only = 1; + } + else if (!strcmp(arg, "-g") || + !strcmp(arg, "--walk-reflogs")) { + init_reflog_walk(&revs->reflog_info); + } + else if (!strcmp(arg, "--default")) { + if (argc <= 1) + return error("bad --default argument"); + revs->def = argv[1]; + return 2; + } + else if (!strcmp(arg, "--merge")) { + revs->show_merge = 1; + } + else if (!strcmp(arg, "--topo-order")) { + revs->lifo = 1; + revs->topo_order = 1; + } + else if (!strcmp(arg, "--date-order")) { + revs->lifo = 0; + revs->topo_order = 1; + } + else if (!prefixcmp(arg, "--early-output")) { + int count = 100; + switch (arg[14]) { + case '=': + count = atoi(arg+15); + /* Fallthrough */ + case 0: + revs->topo_order = 1; + revs->early_output = count; + } + } + else if (!strcmp(arg, "--parents")) { + revs->rewrite_parents = 1; + revs->print_parents = 1; + } + else if (!strcmp(arg, "--dense")) { + revs->dense = 1; + } + else if (!strcmp(arg, "--sparse")) { + revs->dense = 0; + } + else if (!strcmp(arg, "--show-all")) { + revs->show_all = 1; + } + else if (!strcmp(arg, "--remove-empty")) { + revs->remove_empty_trees = 1; + } + else if (!strcmp(arg, "--no-merges")) { + revs->no_merges = 1; + } + else if (!strcmp(arg, "--boundary")) { + revs->boundary = 1; + } + else if (!strcmp(arg, "--left-right")) { + revs->left_right = 1; + } + else if (!strcmp(arg, "--cherry-pick")) { + revs->cherry_pick = 1; + revs->limited = 1; + } + else if (!strcmp(arg, "--objects")) { + revs->tag_objects = 1; + revs->tree_objects = 1; + revs->blob_objects = 1; + } + else if (!strcmp(arg, "--objects-edge")) { + revs->tag_objects = 1; + revs->tree_objects = 1; + revs->blob_objects = 1; + revs->edge_hint = 1; + } + else if (!strcmp(arg, "--unpacked")) { + revs->unpacked = 1; + free(revs->ignore_packed); + revs->ignore_packed = NULL; + revs->num_ignore_packed = 0; + } + else if (!prefixcmp(arg, "--unpacked=")) { + revs->unpacked = 1; + add_ignore_packed(revs, arg+11); + } + else if (!strcmp(arg, "-r")) { + revs->diff = 1; + DIFF_OPT_SET(&revs->diffopt, RECURSIVE); + } + else if (!strcmp(arg, "-t")) { + revs->diff = 1; + DIFF_OPT_SET(&revs->diffopt, RECURSIVE); + DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE); + } + else if (!strcmp(arg, "-m")) { + revs->ignore_merges = 0; + } + else if (!strcmp(arg, "-c")) { + revs->diff = 1; + revs->dense_combined_merges = 0; + revs->combine_merges = 1; + } + else if (!strcmp(arg, "--cc")) { + revs->diff = 1; + revs->dense_combined_merges = 1; + revs->combine_merges = 1; + } + else if (!strcmp(arg, "-v")) { + revs->verbose_header = 1; + } + else if (!strcmp(arg, "--pretty")) { + revs->verbose_header = 1; + get_commit_format(arg+8, revs); + } + else if (!prefixcmp(arg, "--pretty=")) { + revs->verbose_header = 1; + get_commit_format(arg+9, revs); + } + else if (!strcmp(arg, "--graph")) { + revs->topo_order = 1; + revs->rewrite_parents = 1; + revs->graph = graph_init(revs); + } + else if (!strcmp(arg, "--root")) { + revs->show_root_diff = 1; + } + else if (!strcmp(arg, "--no-commit-id")) { + revs->no_commit_id = 1; + } + else if (!strcmp(arg, "--always")) { + revs->always_show_header = 1; + } + else if (!strcmp(arg, "--no-abbrev")) { + revs->abbrev = 0; + } + else if (!strcmp(arg, "--abbrev")) { + revs->abbrev = DEFAULT_ABBREV; + } + else if (!prefixcmp(arg, "--abbrev=")) { + revs->abbrev = strtoul(arg + 9, NULL, 10); + if (revs->abbrev < MINIMUM_ABBREV) + revs->abbrev = MINIMUM_ABBREV; + else if (revs->abbrev > 40) + revs->abbrev = 40; + } + else if (!strcmp(arg, "--abbrev-commit")) { + revs->abbrev_commit = 1; + } + else if (!strcmp(arg, "--full-diff")) { + revs->diff = 1; + revs->full_diff = 1; + } + else if (!strcmp(arg, "--full-history")) { + revs->simplify_history = 0; + } + else if (!strcmp(arg, "--relative-date")) { + revs->date_mode = DATE_RELATIVE; + } + else if (!strncmp(arg, "--date=", 7)) { + revs->date_mode = parse_date_format(arg + 7); + } + else if (!strcmp(arg, "--log-size")) { + revs->show_log_size = 1; + } + /* + * Grepping the commit log + */ + else if (!prefixcmp(arg, "--author=")) { + add_header_grep(revs, "author", arg+9); + } + else if (!prefixcmp(arg, "--committer=")) { + add_header_grep(revs, "committer", arg+12); + } + else if (!prefixcmp(arg, "--grep=")) { + add_message_grep(revs, arg+7); + } + else if (!strcmp(arg, "--extended-regexp") || + !strcmp(arg, "-E")) { + if (revs->grep_filter) + revs->grep_filter->regflags |= REG_EXTENDED; + } + else if (!strcmp(arg, "--regexp-ignore-case") || + !strcmp(arg, "-i")) { + if (revs->grep_filter) + revs->grep_filter->regflags |= REG_ICASE; + } + else if (!strcmp(arg, "--fixed-strings") || + !strcmp(arg, "-F")) { + if (revs->grep_filter) + revs->grep_filter->fixed = 1; + } + else if (!strcmp(arg, "--all-match")) { + if (revs->grep_filter) + revs->grep_filter->all_match = 1; + } + else if (!prefixcmp(arg, "--encoding=")) { + arg += 11; + if (strcmp(arg, "none")) + git_log_output_encoding = xstrdup(arg); + else + git_log_output_encoding = ""; + } + else if (!strcmp(arg, "--reverse")) { + revs->reverse ^= 1; + } + else if (!strcmp(arg, "--children")) { + revs->children.name = "children"; + revs->limited = 1; + } else { + int opts = diff_opt_parse(&revs->diffopt, argv, argc); + if (!opts) + unkv[(*unkc)++] = arg; + return opts; + } + + return 1; +} + /* * Parse revision information, filling in the "rev_info" structure, * and removing the used arguments from the argument list. @@ -983,12 +1253,7 @@ static void add_ignore_packed(struct rev_info *revs, const char *name) */ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def) { - int i, flags, seen_dashdash, show_merge; - const char **unrecognized = argv + 1; - int left = 1; - int all_match = 0; - int regflags = 0; - int fixed = 0; + int i, flags, left, seen_dashdash; /* First, search for "--" */ seen_dashdash = 0; @@ -1004,58 +1269,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch break; } - flags = show_merge = 0; - for (i = 1; i < argc; i++) { + /* Second, deal with arguments and options */ + flags = 0; + for (left = i = 1; i < argc; i++) { const char *arg = argv[i]; if (*arg == '-') { int opts; - if (!prefixcmp(arg, "--max-count=")) { - revs->max_count = atoi(arg + 12); - continue; - } - if (!prefixcmp(arg, "--skip=")) { - revs->skip_count = atoi(arg + 7); - continue; - } - /* accept -<digit>, like traditional "head" */ - if ((*arg == '-') && isdigit(arg[1])) { - revs->max_count = atoi(arg + 1); - continue; - } - if (!strcmp(arg, "-n")) { - if (argc <= i + 1) - die("-n requires an argument"); - revs->max_count = atoi(argv[++i]); - continue; - } - if (!prefixcmp(arg, "-n")) { - revs->max_count = atoi(arg + 2); - continue; - } - if (!prefixcmp(arg, "--max-age=")) { - revs->max_age = atoi(arg + 10); - continue; - } - if (!prefixcmp(arg, "--since=")) { - revs->max_age = approxidate(arg + 8); - continue; - } - if (!prefixcmp(arg, "--after=")) { - revs->max_age = approxidate(arg + 8); - continue; - } - if (!prefixcmp(arg, "--min-age=")) { - revs->min_age = atoi(arg + 10); - continue; - } - if (!prefixcmp(arg, "--before=")) { - revs->min_age = approxidate(arg + 9); - continue; - } - if (!prefixcmp(arg, "--until=")) { - revs->min_age = approxidate(arg + 8); - continue; - } + if (!strcmp(arg, "--all")) { handle_refs(revs, flags, for_each_ref); continue; @@ -1072,265 +1292,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch handle_refs(revs, flags, for_each_remote_ref); continue; } - if (!strcmp(arg, "--first-parent")) { - revs->first_parent_only = 1; - continue; - } if (!strcmp(arg, "--reflog")) { handle_reflog(revs, flags); continue; } - if (!strcmp(arg, "-g") || - !strcmp(arg, "--walk-reflogs")) { - init_reflog_walk(&revs->reflog_info); - continue; - } if (!strcmp(arg, "--not")) { flags ^= UNINTERESTING; continue; } - if (!strcmp(arg, "--default")) { - if (++i >= argc) - die("bad --default argument"); - def = argv[i]; - continue; - } - if (!strcmp(arg, "--merge")) { - show_merge = 1; - continue; - } - if (!strcmp(arg, "--topo-order")) { - revs->lifo = 1; - revs->topo_order = 1; - continue; - } - if (!strcmp(arg, "--date-order")) { - revs->lifo = 0; - revs->topo_order = 1; - continue; - } - if (!prefixcmp(arg, "--early-output")) { - int count = 100; - switch (arg[14]) { - case '=': - count = atoi(arg+15); - /* Fallthrough */ - case 0: - revs->topo_order = 1; - revs->early_output = count; - continue; - } - } - if (!strcmp(arg, "--parents")) { - revs->rewrite_parents = 1; - revs->print_parents = 1; - continue; - } - if (!strcmp(arg, "--dense")) { - revs->dense = 1; - continue; - } - if (!strcmp(arg, "--sparse")) { - revs->dense = 0; - continue; - } - if (!strcmp(arg, "--show-all")) { - revs->show_all = 1; - continue; - } - if (!strcmp(arg, "--remove-empty")) { - revs->remove_empty_trees = 1; - continue; - } - if (!strcmp(arg, "--no-merges")) { - revs->no_merges = 1; - continue; - } - if (!strcmp(arg, "--boundary")) { - revs->boundary = 1; - continue; - } - if (!strcmp(arg, "--left-right")) { - revs->left_right = 1; - continue; - } - if (!strcmp(arg, "--cherry-pick")) { - revs->cherry_pick = 1; - revs->limited = 1; - continue; - } - if (!strcmp(arg, "--objects")) { - revs->tag_objects = 1; - revs->tree_objects = 1; - revs->blob_objects = 1; - continue; - } - if (!strcmp(arg, "--objects-edge")) { - revs->tag_objects = 1; - revs->tree_objects = 1; - revs->blob_objects = 1; - revs->edge_hint = 1; - continue; - } - if (!strcmp(arg, "--unpacked")) { - revs->unpacked = 1; - free(revs->ignore_packed); - revs->ignore_packed = NULL; - revs->num_ignore_packed = 0; - continue; - } - if (!prefixcmp(arg, "--unpacked=")) { - revs->unpacked = 1; - add_ignore_packed(revs, arg+11); - continue; - } - if (!strcmp(arg, "-r")) { - revs->diff = 1; - DIFF_OPT_SET(&revs->diffopt, RECURSIVE); - continue; - } - if (!strcmp(arg, "-t")) { - revs->diff = 1; - DIFF_OPT_SET(&revs->diffopt, RECURSIVE); - DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE); - continue; - } - if (!strcmp(arg, "-m")) { - revs->ignore_merges = 0; - continue; - } - if (!strcmp(arg, "-c")) { - revs->diff = 1; - revs->dense_combined_merges = 0; - 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; - continue; - } - if (!strcmp(arg, "--pretty")) { - revs->verbose_header = 1; - get_commit_format(arg+8, revs); - continue; - } - if (!prefixcmp(arg, "--pretty=")) { - revs->verbose_header = 1; - get_commit_format(arg+9, revs); - continue; - } - if (!strcmp(arg, "--graph")) { - revs->topo_order = 1; - revs->rewrite_parents = 1; - revs->graph = graph_init(revs); - 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 (!prefixcmp(arg, "--abbrev=")) { - revs->abbrev = strtoul(arg + 9, NULL, 10); - if (revs->abbrev < MINIMUM_ABBREV) - revs->abbrev = MINIMUM_ABBREV; - else if (revs->abbrev > 40) - revs->abbrev = 40; - continue; - } - if (!strcmp(arg, "--abbrev-commit")) { - revs->abbrev_commit = 1; - continue; - } - if (!strcmp(arg, "--full-diff")) { - revs->diff = 1; - revs->full_diff = 1; - continue; - } - if (!strcmp(arg, "--full-history")) { - revs->simplify_history = 0; - continue; - } - if (!strcmp(arg, "--relative-date")) { - revs->date_mode = DATE_RELATIVE; - continue; - } - if (!strncmp(arg, "--date=", 7)) { - revs->date_mode = parse_date_format(arg + 7); - continue; - } - if (!strcmp(arg, "--log-size")) { - revs->show_log_size = 1; - continue; - } - - /* - * Grepping the commit log - */ - if (!prefixcmp(arg, "--author=")) { - add_header_grep(revs, "author", arg+9); - continue; - } - if (!prefixcmp(arg, "--committer=")) { - add_header_grep(revs, "committer", arg+12); - continue; - } - if (!prefixcmp(arg, "--grep=")) { - add_message_grep(revs, arg+7); - continue; - } - if (!strcmp(arg, "--extended-regexp") || - !strcmp(arg, "-E")) { - regflags |= REG_EXTENDED; - continue; - } - if (!strcmp(arg, "--regexp-ignore-case") || - !strcmp(arg, "-i")) { - regflags |= REG_ICASE; - continue; - } - if (!strcmp(arg, "--fixed-strings") || - !strcmp(arg, "-F")) { - fixed = 1; - continue; - } - if (!strcmp(arg, "--all-match")) { - all_match = 1; - continue; - } - if (!prefixcmp(arg, "--encoding=")) { - arg += 11; - if (strcmp(arg, "none")) - git_log_output_encoding = xstrdup(arg); - else - git_log_output_encoding = ""; - continue; - } - if (!strcmp(arg, "--reverse")) { - revs->reverse ^= 1; - continue; - } if (!strcmp(arg, "--no-walk")) { revs->no_walk = 1; continue; @@ -1339,19 +1308,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch revs->no_walk = 0; continue; } - if (!strcmp(arg, "--children")) { - revs->children.name = "children"; - revs->limited = 1; - continue; - } - opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i); + opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv); if (opts > 0) { i += opts - 1; continue; } - *unrecognized++ = arg; - left++; + if (opts < 0) + exit(128); continue; } @@ -1375,21 +1339,18 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch } } - if (revs->grep_filter) { - revs->grep_filter->regflags |= regflags; - revs->grep_filter->fixed = fixed; - } - - if (show_merge) + if (revs->def == NULL) + revs->def = def; + if (revs->show_merge) prepare_show_merge(revs); - if (def && !revs->pending.nr) { + if (revs->def && !revs->pending.nr) { unsigned char sha1[20]; struct object *object; unsigned mode; - if (get_sha1_with_mode(def, sha1, &mode)) - die("bad default revision '%s'", def); - object = get_reference(revs, def, sha1, 0); - add_pending_object_with_mode(revs, object, def, mode); + if (get_sha1_with_mode(revs->def, sha1, &mode)) + die("bad default revision '%s'", revs->def); + object = get_reference(revs, revs->def, sha1, 0); + add_pending_object_with_mode(revs, object, revs->def, mode); } /* Did the user ask for any diff output? Run the diff! */ @@ -1423,7 +1384,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch die("diff_setup_done failed"); if (revs->grep_filter) { - revs->grep_filter->all_match = all_match; compile_grep_patterns(revs->grep_filter); } diff --git a/revision.h b/revision.h index 5b8c56b..cc80fcd 100644 --- a/revision.h +++ b/revision.h @@ -26,6 +26,7 @@ struct rev_info { /* Basic information */ const char *prefix; + const char *def; void *prune_data; unsigned int early_output; @@ -66,6 +67,7 @@ struct rev_info { /* Format info */ unsigned int shown_one:1, + show_merge:1, abbrev_commit:1, use_terminator:1, missing_newline:1; @@ -120,6 +122,8 @@ volatile show_early_output_fn_t show_early_output; extern void init_revisions(struct rev_info *revs, const char *prefix); extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def); +extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv, + int *unkc, const char **unkv); extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename); extern int prepare_revision_walk(struct rev_info *revs); -- 1.5.6.2.396.gca539 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] git-blame: migrate to incremental parse-option [1/2] 2008-07-08 13:19 ` [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions Pierre Habouzit @ 2008-07-08 13:19 ` Pierre Habouzit 2008-07-08 13:19 ` [PATCH 3/3] git-blame: migrate to incremental parse-option [2/2] Pierre Habouzit 0 siblings, 1 reply; 6+ messages in thread From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw) To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit This step merely moves the parser to an incremental version, still using parse_revisions. Signed-off-by: Pierre Habouzit <madcoder@debian•org> --- builtin-blame.c | 220 +++++++++++++++++++++++++++++------------------------- 1 files changed, 118 insertions(+), 102 deletions(-) diff --git a/builtin-blame.c b/builtin-blame.c index cf41511..5e82cd3 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -18,24 +18,16 @@ #include "cache-tree.h" #include "path-list.h" #include "mailmap.h" +#include "parse-options.h" -static char blame_usage[] = -"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n" -" -c Use the same output mode as git-annotate (Default: off)\n" -" -b Show blank SHA-1 for boundary commits (Default: off)\n" -" -l Show long commit SHA1 (Default: off)\n" -" --root Do not treat root commits as boundaries (Default: off)\n" -" -t Show raw timestamp (Default: off)\n" -" -f, --show-name Show original filename (Default: auto)\n" -" -n, --show-number Show original linenumber (Default: off)\n" -" -s Suppress author name and timestamp (Default: off)\n" -" -p, --porcelain Show in a format designed for machine consumption\n" -" -w Ignore whitespace differences\n" -" -L n,m Process only line range n,m, counting from 1\n" -" -M, -C Find line movements within and across files\n" -" --incremental Show blame entries as we find them, incrementally\n" -" --contents file Use <file>'s contents as the final image\n" -" -S revs-file Use revisions from revs-file instead of calling git-rev-list\n"; +static char blame_usage[] = "git-blame [options] [rev-opts] [rev] [--] file"; + +static const char *blame_opt_usage[] = { + blame_usage, + "", + "[rev-opts] are documented in git-rev-parse(1)", + NULL +}; static int longest_file; static int longest_author; @@ -2219,6 +2211,50 @@ static const char *prepare_initial(struct scoreboard *sb) return final_commit_name; } +static int blame_copy_callback(const struct option *option, const char *arg, int unset) +{ + int *opt = option->value; + + /* + * -C enables copy from removed files; + * -C -C enables copy from existing files, but only + * when blaming a new file; + * -C -C -C enables copy from existing files for + * everybody + */ + if (*opt & PICKAXE_BLAME_COPY_HARDER) + *opt |= PICKAXE_BLAME_COPY_HARDEST; + if (*opt & PICKAXE_BLAME_COPY) + *opt |= PICKAXE_BLAME_COPY_HARDER; + *opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE; + + if (arg) + blame_copy_score = parse_score(arg); + return 0; +} + +static int blame_move_callback(const struct option *option, const char *arg, int unset) +{ + int *opt = option->value; + + *opt |= PICKAXE_BLAME_MOVE; + + if (arg) + blame_move_score = parse_score(arg); + return 0; +} + +static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset) +{ + const char **bottomtop = option->value; + if (!arg) + return -1; + if (*bottomtop) + die("More than one '-L n,m' option given"); + *bottomtop = arg; + return 0; +} + int cmd_blame(int argc, const char **argv, const char *prefix) { struct rev_info revs; @@ -2226,98 +2262,79 @@ int cmd_blame(int argc, const char **argv, const char *prefix) struct scoreboard sb; struct origin *o; struct blame_entry *ent; - int i, seen_dashdash, unk, opt; + int i, seen_dashdash, unk; long bottom, top, lno; - int output_option = 0; - int show_stats = 0; - const char *revs_file = NULL; const char *final_commit_name = NULL; enum object_type type; - const char *bottomtop = NULL; - const char *contents_from = NULL; + + static const char *bottomtop = NULL; + static int output_option = 0, opt = 0; + static int show_stats = 0; + static const char *revs_file = NULL; + static const char *contents_from = NULL; + static const struct option options[] = { + OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"), + OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"), + OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"), + OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"), + OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE), + OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME), + OPT_BIT('n', "show-number", &output_option, "Show original linenumber (Default: off)", OUTPUT_SHOW_NUMBER), + OPT_BIT('p', "porcelain", &output_option, "Show in a format designed for machine consumption", OUTPUT_PORCELAIN), + OPT_BIT('c', NULL, &output_option, "Use the same output mode as git-annotate (Default: off)", OUTPUT_ANNOTATE_COMPAT), + OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP), + OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME), + OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR), + OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE), + OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"), + OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"), + { OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback }, + { OPTION_CALLBACK, 'M', NULL, &opt, "score", "Find line movements within and across files", PARSE_OPT_OPTARG, blame_move_callback }, + OPT_CALLBACK('L', NULL, &bottomtop, "n,m", "Process only line range n,m, counting from 1", blame_bottomtop_callback), + OPT_END() + }; + + struct parse_opt_ctx_t ctx; cmd_is_annotate = !strcmp(argv[0], "annotate"); git_config(git_blame_config, NULL); + init_revisions(&revs, NULL); save_commit_buffer = 0; - opt = 0; + parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH | + PARSE_OPT_KEEP_ARGV0); + for (;;) { + int n; + + switch (parse_options_step(&ctx, options, blame_opt_usage)) { + case PARSE_OPT_HELP: + exit(129); + case PARSE_OPT_DONE: + goto parse_done; + } + + if (!strcmp(ctx.argv[0], "--reverse")) { + ctx.argv[0] = "--children"; + reverse = 1; + } + n = handle_revision_opt(&revs, ctx.argc, ctx.argv, + &ctx.cpidx, ctx.out); + if (n <= 0) { + error("unknown option `%s'", ctx.argv[0]); + usage_with_options(blame_opt_usage, options); + } + ctx.argv += n; + ctx.argc -= n; + } +parse_done: + argc = parse_options_end(&ctx); + seen_dashdash = 0; for (unk = i = 1; i < argc; i++) { const char *arg = argv[i]; if (*arg != '-') break; - else if (!strcmp("-b", arg)) - blank_boundary = 1; - else if (!strcmp("--root", arg)) - show_root = 1; - else if (!strcmp("--reverse", arg)) { - argv[unk++] = "--children"; - reverse = 1; - } - else if (!strcmp(arg, "--show-stats")) - show_stats = 1; - else if (!strcmp("-c", arg)) - output_option |= OUTPUT_ANNOTATE_COMPAT; - else if (!strcmp("-t", arg)) - output_option |= OUTPUT_RAW_TIMESTAMP; - else if (!strcmp("-l", arg)) - output_option |= OUTPUT_LONG_OBJECT_NAME; - else if (!strcmp("-s", arg)) - output_option |= OUTPUT_NO_AUTHOR; - else if (!strcmp("-w", arg)) - xdl_opts |= XDF_IGNORE_WHITESPACE; - else if (!strcmp("-S", arg) && ++i < argc) - revs_file = argv[i]; - else if (!prefixcmp(arg, "-M")) { - opt |= PICKAXE_BLAME_MOVE; - blame_move_score = parse_score(arg+2); - } - else if (!prefixcmp(arg, "-C")) { - /* - * -C enables copy from removed files; - * -C -C enables copy from existing files, but only - * when blaming a new file; - * -C -C -C enables copy from existing files for - * everybody - */ - if (opt & PICKAXE_BLAME_COPY_HARDER) - opt |= PICKAXE_BLAME_COPY_HARDEST; - if (opt & PICKAXE_BLAME_COPY) - opt |= PICKAXE_BLAME_COPY_HARDER; - opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE; - blame_copy_score = parse_score(arg+2); - } - else if (!prefixcmp(arg, "-L")) { - if (!arg[2]) { - if (++i >= argc) - usage(blame_usage); - arg = argv[i]; - } - else - arg += 2; - if (bottomtop) - die("More than one '-L n,m' option given"); - bottomtop = arg; - } - else if (!strcmp("--contents", arg)) { - if (++i >= argc) - usage(blame_usage); - contents_from = argv[i]; - } - else if (!strcmp("--incremental", arg)) - incremental = 1; - else if (!strcmp("--score-debug", arg)) - output_option |= OUTPUT_SHOW_SCORE; - else if (!strcmp("-f", arg) || - !strcmp("--show-name", arg)) - output_option |= OUTPUT_SHOW_NAME; - else if (!strcmp("-n", arg) || - !strcmp("--show-number", arg)) - output_option |= OUTPUT_SHOW_NUMBER; - else if (!strcmp("-p", arg) || - !strcmp("--porcelain", arg)) - output_option |= OUTPUT_PORCELAIN; else if (!strcmp("--", arg)) { seen_dashdash = 1; i++; @@ -2364,16 +2381,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix) if (seen_dashdash) { /* (1) */ if (argc <= i) - usage(blame_usage); + usage_with_options(blame_opt_usage, options); path = add_prefix(prefix, argv[i]); if (i + 1 == argc - 1) { if (unk != 1) - usage(blame_usage); + usage_with_options(blame_opt_usage, options); argv[unk++] = argv[i + 1]; } else if (i + 1 != argc) /* garbage at end */ - usage(blame_usage); + usage_with_options(blame_opt_usage, options); } else { int j; @@ -2383,7 +2400,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) if (seen_dashdash) { /* (2) */ if (seen_dashdash + 1 != argc - 1) - usage(blame_usage); + usage_with_options(blame_opt_usage, options); path = add_prefix(prefix, argv[seen_dashdash + 1]); for (j = i; j < seen_dashdash; j++) argv[unk++] = argv[j]; @@ -2391,7 +2408,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) else { /* (3) */ if (argc <= i) - usage(blame_usage); + usage_with_options(blame_opt_usage, options); path = add_prefix(prefix, argv[i]); if (i + 1 == argc - 1) { final_commit_name = argv[i + 1]; @@ -2405,7 +2422,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) } } else if (i != argc - 1) - usage(blame_usage); /* garbage at end */ + usage_with_options(blame_opt_usage, options); setup_work_tree(); if (!has_path_in_work_tree(path)) @@ -2424,7 +2441,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix) argv[unk++] = "--"; /* terminate the rev name */ argv[unk] = NULL; - init_revisions(&revs, NULL); setup_revisions(unk, argv, &revs, NULL); memset(&sb, 0, sizeof(sb)); -- 1.5.6.2.396.gca539 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] git-blame: migrate to incremental parse-option [2/2] 2008-07-08 13:19 ` [PATCH 2/3] git-blame: migrate to incremental parse-option [1/2] Pierre Habouzit @ 2008-07-08 13:19 ` Pierre Habouzit 0 siblings, 0 replies; 6+ messages in thread From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw) To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit Now use handle_revision_args instead of parse_revisions, and simplify the handling of old-style arguments a lot thanks to the filtering. Signed-off-by: Pierre Habouzit <madcoder@debian•org> --- builtin-blame.c | 130 ++++++++++++++++--------------------------------------- 1 files changed, 38 insertions(+), 92 deletions(-) diff --git a/builtin-blame.c b/builtin-blame.c index 5e82cd3..99f5140 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -2262,8 +2262,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) struct scoreboard sb; struct origin *o; struct blame_entry *ent; - int i, seen_dashdash, unk; - long bottom, top, lno; + long dashdash_pos, bottom, top, lno; const char *final_commit_name = NULL; enum object_type type; @@ -2301,6 +2300,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) git_config(git_blame_config, NULL); init_revisions(&revs, NULL); save_commit_buffer = 0; + dashdash_pos = 0; parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0); @@ -2311,6 +2311,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix) case PARSE_OPT_HELP: exit(129); case PARSE_OPT_DONE: + if (ctx.argv[0]) + dashdash_pos = ctx.cpidx; goto parse_done; } @@ -2330,20 +2332,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix) parse_done: argc = parse_options_end(&ctx); - seen_dashdash = 0; - for (unk = i = 1; i < argc; i++) { - const char *arg = argv[i]; - if (*arg != '-') - break; - else if (!strcmp("--", arg)) { - seen_dashdash = 1; - i++; - break; - } - else - argv[unk++] = arg; - } - if (!blame_move_score) blame_move_score = BLAME_DEFAULT_MOVE_SCORE; if (!blame_copy_score) @@ -2356,92 +2344,50 @@ parse_done: * * The remaining are: * - * (1) if seen_dashdash, its either - * "-options -- <path>" or - * "-options -- <path> <rev>". - * but the latter is allowed only if there is no - * options that we passed to revision machinery. + * (1) if dashdash_pos != 0, its either + * "blame [revisions] -- <path>" or + * "blame -- <path> <rev>" * - * (2) otherwise, we may have "--" somewhere later and - * might be looking at the first one of multiple 'rev' - * parameters (e.g. " master ^next ^maint -- path"). - * See if there is a dashdash first, and give the - * arguments before that to revision machinery. - * After that there must be one 'path'. + * (2) otherwise, its one of the two: + * "blame [revisions] <path>" + * "blame <path> <rev>" * - * (3) otherwise, its one of the three: - * "-options <path> <rev>" - * "-options <rev> <path>" - * "-options <path>" - * but again the first one is allowed only if - * there is no options that we passed to revision - * machinery. + * Note that we must strip out <path> from the arguments: we do not + * want the path pruning but we may want "bottom" processing. */ - - if (seen_dashdash) { - /* (1) */ - if (argc <= i) - usage_with_options(blame_opt_usage, options); - path = add_prefix(prefix, argv[i]); - if (i + 1 == argc - 1) { - if (unk != 1) + if (dashdash_pos) { + switch (argc - dashdash_pos - 1) { + case 2: /* (1b) */ + if (argc != 4) usage_with_options(blame_opt_usage, options); - argv[unk++] = argv[i + 1]; + /* reorder for the new way: <rev> -- <path> */ + argv[1] = argv[3]; + argv[3] = argv[2]; + argv[2] = "--"; + /* FALLTHROUGH */ + case 1: /* (1a) */ + path = add_prefix(prefix, argv[--argc]); + argv[argc] = NULL; + break; + default: + usage_with_options(blame_opt_usage, options); } - else if (i + 1 != argc) - /* garbage at end */ + } else { + if (argc < 2) usage_with_options(blame_opt_usage, options); - } - else { - int j; - for (j = i; !seen_dashdash && j < argc; j++) - if (!strcmp(argv[j], "--")) - seen_dashdash = j; - if (seen_dashdash) { - /* (2) */ - if (seen_dashdash + 1 != argc - 1) - usage_with_options(blame_opt_usage, options); - path = add_prefix(prefix, argv[seen_dashdash + 1]); - for (j = i; j < seen_dashdash; j++) - argv[unk++] = argv[j]; + path = add_prefix(prefix, argv[argc - 1]); + if (argc == 3 && !has_path_in_work_tree(path)) { /* (2b) */ + path = add_prefix(prefix, argv[1]); + argv[1] = argv[2]; } - else { - /* (3) */ - if (argc <= i) - usage_with_options(blame_opt_usage, options); - path = add_prefix(prefix, argv[i]); - if (i + 1 == argc - 1) { - final_commit_name = argv[i + 1]; - - /* if (unk == 1) we could be getting - * old-style - */ - if (unk == 1 && !has_path_in_work_tree(path)) { - path = add_prefix(prefix, argv[i + 1]); - final_commit_name = argv[i]; - } - } - else if (i != argc - 1) - usage_with_options(blame_opt_usage, options); + argv[argc - 1] = "--"; - setup_work_tree(); - if (!has_path_in_work_tree(path)) - die("cannot stat path %s: %s", - path, strerror(errno)); - } + setup_work_tree(); + if (!has_path_in_work_tree(path)) + die("cannot stat path %s: %s", path, strerror(errno)); } - if (final_commit_name) - argv[unk++] = final_commit_name; - - /* - * Now we got rev and path. We do not want the path pruning - * but we may want "bottom" processing. - */ - argv[unk++] = "--"; /* terminate the rev name */ - argv[unk] = NULL; - - setup_revisions(unk, argv, &revs, NULL); + setup_revisions(argc, argv, &revs, NULL); memset(&sb, 0, sizeof(sb)); sb.revs = &revs; -- 1.5.6.2.396.gca539 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [take 2] revision parsing made incremental 2008-07-08 13:19 [take 2] revision parsing made incremental Pierre Habouzit 2008-07-08 13:19 ` [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions Pierre Habouzit @ 2008-07-08 17:41 ` Linus Torvalds 2008-07-08 22:57 ` Junio C Hamano 2 siblings, 0 replies; 6+ messages in thread From: Linus Torvalds @ 2008-07-08 17:41 UTC (permalink / raw) To: Pierre Habouzit; +Cc: git, gitster, peff, Johannes.Schindelin On Tue, 8 Jul 2008, Pierre Habouzit wrote: > > The series passes the testsuite, has no know blanks issues, and is > pushed to my public repository. Ack. Looks good to me. Linus ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [take 2] revision parsing made incremental 2008-07-08 13:19 [take 2] revision parsing made incremental Pierre Habouzit 2008-07-08 13:19 ` [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions Pierre Habouzit 2008-07-08 17:41 ` [take 2] revision parsing made incremental Linus Torvalds @ 2008-07-08 22:57 ` Junio C Hamano 2 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2008-07-08 22:57 UTC (permalink / raw) To: Pierre Habouzit; +Cc: git, torvalds, peff, Johannes.Schindelin Pierre Habouzit <madcoder@debian•org> writes: > Following Dscho's remarks, I reworked the series to avoid changing > setup_revisions semantics for now, and only exposed the part that groks > options (and keep pseudo revision arguments out). > > It indeed makes the series smaller, even if the first patch is quite > long to read, and is just enough for simplifying git-blame in a very > satisfying way. All looks good and the result looks much more readable. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-07-08 22:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-07-08 13:19 [take 2] revision parsing made incremental Pierre Habouzit 2008-07-08 13:19 ` [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions Pierre Habouzit 2008-07-08 13:19 ` [PATCH 2/3] git-blame: migrate to incremental parse-option [1/2] Pierre Habouzit 2008-07-08 13:19 ` [PATCH 3/3] git-blame: migrate to incremental parse-option [2/2] Pierre Habouzit 2008-07-08 17:41 ` [take 2] revision parsing made incremental Linus Torvalds 2008-07-08 22:57 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox