{"thread":{"id":"14344","subject":"Migration of builtin-blame to parse-option","startedAt":"2008-07-08T09:55:58Z","lastAt":"2008-07-08T12:48:32Z","messageCount":15,"participants":["Pierre Habouzit","Johannes Sixt","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"82571","messageId":"1215510964-16664-1-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":null,"subject":"Migration of builtin-blame to parse-option","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:55:58Z","receivedAt":"2008-07-08T09:55:58Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"This series is twofold. The first 4 patches refactor the revision\nparsing machine so that it's easier to deal with from a parse-opt based\nparser:\n\n[PATCH 1/6] revisions: refactor init_revisions and setup_revisions.\n21 files changed, 70 insertions(+), 54 deletions(-)\n\n  This is the patch I sent before, revisited.\n\n[PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.\n2 files changed, 273 insertions(+), 308 deletions(-)\n\n  This patch splits out the revisions _option_ parsing (as opposed to\n  revisions arguments or pseudo arguments like --all, --not, ...) from\n  the revision parser. The patch is huge, but quite straightforward.\n\n[PATCH 3/6] revisions: parse_revisions refactor.\n1 files changed, 38 insertions(+), 25 deletions(-)\n\n  This patch reworks parse_revisions so that it works internally as if\n  it was a parse-opt parser. It's equivalent to the previous code, but\n  changes are tricky, and a separate commit is really worth it.\n\n[PATCH 4/6] revisions: split handle_revision_args from parse_revisions.\n2 files changed, 52 insertions(+), 46 deletions(-)\n\n  This patch splits out the last bit of parse-revisions (from the\n  previous commit) so that what parses revisions arguments (refs and\n  pseudo arguments like --all/--not/...) can be called independently.\n\n  The commit is straightforward but moves code around, hence the\n  separate commit from patch 3 so that one isn't lost in the code moves\n  while reviewing the tricky bits from patch 3.\n\n\nThe second part is the git-blame migration for real:\n\n[PATCH 5/6] git-blame: migrate to incremental parse-option [1/2]\n1 files changed, 118 insertions(+), 102 deletions(-)\n\n  This bit is really alike the proof of concept I saw. Most of the code\n  is stolen from Linus initial patch. It merely removes the git-blame\n  own options and deal with them with a parse-opt parser, in an\n  incremental way.\n\n  All in all, it's a pretty straighforward patch.\n\n[PATCH 6/6] git-blame: migrate to incremental parse-option [2/2]\n1 files changed, 40 insertions(+), 92 deletions(-)\n\n  This is by far the trickiest patch of the series, though it passes the\n  testsuite fine. This patch uses the function from patch 4 to process\n  the revision arguments. We know the dashdash position from the\n  incremental parse-opt, but we have to deal with the \"old way\" of\n  passing arguments to git-blame.\n\n  The new code is really less involved than before, because we deal with\n  a filtered argv array where we only have the revisions arguments (no\n  options anymore) and the <path>.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82575","messageId":"1215510964-16664-2-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":"1215510964-16664-1-git-send-email-madcoder@debian.org","subject":"[PATCH 1/6] revisions: refactor init_revisions and setup_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:55:59Z","receivedAt":"2008-07-08T09:55:59Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"struct rev_info gains two new field:\n* .def to store --default argument;\n* .show_merge 1-bit field.\n\nsetup_revisions has been split in two: parse_revisions that does (almost)\nonly argument parsing, to be more like what parse-options can do, and\nsetup_revisions that does the rest.\n\nMany places had no arguments to pass to setup_revisions, and those don't use\nparse_revisions at all.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n builtin-add.c           |    2 +-\n builtin-blame.c         |    3 +-\n builtin-commit.c        |    4 +-\n builtin-diff-files.c    |    3 +-\n builtin-diff-index.c    |    3 +-\n builtin-diff-tree.c     |    3 +-\n builtin-diff.c          |    3 +-\n builtin-fast-export.c   |    3 +-\n builtin-fmt-merge-msg.c |    2 +-\n builtin-log.c           |    7 +++--\n builtin-pack-objects.c  |    3 +-\n builtin-rev-list.c      |    3 +-\n builtin-revert.c        |    2 +-\n builtin-shortlog.c      |    3 +-\n bundle.c                |    6 +++-\n http-push.c             |    3 +-\n remote.c                |    3 +-\n revision.c              |   51 +++++++++++++++++++++++------------------------\n revision.h              |    7 ++++-\n upload-pack.c           |    4 +-\n wt-status.c             |    6 ++--\n 21 files changed, 70 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 9930cf5..8ba9604 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -121,7 +121,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n \tstruct update_callback_data data;\n \tstruct rev_info rev;\n \tinit_revisions(&rev, prefix);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n+\tsetup_revisions(&rev, NULL);\n \trev.prune_data = pathspec;\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = update_callback;\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex cf41511..1e26b88 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -2425,7 +2425,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \targv[unk] = NULL;\n \n \tinit_revisions(&revs, NULL);\n-\tsetup_revisions(unk, argv, &revs, NULL);\n+\tparse_revisions(unk, argv, &revs);\n+\tsetup_revisions(&revs, NULL);\n \tmemset(&sb, 0, sizeof(sb));\n \n \tsb.revs = &revs;\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 745c11e..37b4e30 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -605,7 +605,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\telse {\n \t\t\tinit_revisions(&rev, \"\");\n \t\t\trev.abbrev = 0;\n-\t\t\tsetup_revisions(0, NULL, &rev, parent);\n+\t\t\tsetup_revisions(&rev, parent);\n \t\t\tDIFF_OPT_SET(&rev.diffopt, QUIET);\n \t\t\tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n \t\t\trun_diff_index(&rev, 1 /* cached */);\n@@ -850,7 +850,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \t\tdie(\"could not parse newly created commit\");\n \n \tinit_revisions(&rev, prefix);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n+\tsetup_revisions(&rev, NULL);\n \n \trev.abbrev = 0;\n \trev.diff = 1;\ndiff --git a/builtin-diff-files.c b/builtin-diff-files.c\nindex 384d871..e7e202a 100644\n--- a/builtin-diff-files.c\n+++ b/builtin-diff-files.c\n@@ -23,7 +23,8 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n \trev.abbrev = 0;\n \n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\targc = parse_revisions(argc, argv, &rev);\n+\tsetup_revisions(&rev, NULL);\n \twhile (1 < argc && argv[1][0] == '-') {\n \t\tif (!strcmp(argv[1], \"--base\"))\n \t\t\trev.max_count = 1;\ndiff --git a/builtin-diff-index.c b/builtin-diff-index.c\nindex 2f44ebf..286a9de 100644\n--- a/builtin-diff-index.c\n+++ b/builtin-diff-index.c\n@@ -20,7 +20,8 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n \trev.abbrev = 0;\n \n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\targc = parse_revisions(argc, argv, &rev);\n+\tsetup_revisions(&rev, NULL);\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \ndiff --git a/builtin-diff-tree.c b/builtin-diff-tree.c\nindex 9d2a48f..0900175 100644\n--- a/builtin-diff-tree.c\n+++ b/builtin-diff-tree.c\n@@ -72,7 +72,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \tnr_sha1 = 0;\n \topt->abbrev = 0;\n \topt->diff = 1;\n-\targc = setup_revisions(argc, argv, opt, NULL);\n+\targc = parse_revisions(argc, argv, opt);\n+\tsetup_revisions(opt, NULL);\n \n \twhile (--argc > 0) {\n \t\tconst char *arg = *++argv;\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 4c289e7..a92dbd4 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -283,7 +283,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n \tif (nongit)\n \t\tdie(\"Not a git repository\");\n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\targc = parse_revisions(argc, argv, &rev);\n+\tsetup_revisions(&rev, NULL);\n \tif (!rev.diffopt.output_format) {\n \t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \t\tif (diff_setup_done(&rev.diffopt) < 0)\ndiff --git a/builtin-fast-export.c b/builtin-fast-export.c\nindex 75132ba..d6726a8 100644\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -454,7 +454,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tgit_config(git_default_config, NULL);\n \n \tinit_revisions(&revs, prefix);\n-\targc = setup_revisions(argc, argv, &revs, NULL);\n+\targc = parse_revisions(argc, argv, &revs);\n+\tsetup_revisions(&revs, NULL);\n \targc = parse_options(argc, argv, options, fast_export_usage, 0);\n \tif (argc > 1)\n \t\tusage_with_options (fast_export_usage, options);\ndiff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c\nindex 69a8a92..3db10c4 100644\n--- a/builtin-fmt-merge-msg.c\n+++ b/builtin-fmt-merge-msg.c\n@@ -183,7 +183,7 @@ static void shortlog(const char *name, unsigned char *sha1,\n \tif (!branch || branch->type != OBJ_COMMIT)\n \t\treturn;\n \n-\tsetup_revisions(0, NULL, rev, NULL);\n+\tsetup_revisions(rev, NULL);\n \trev->ignore_merges = 1;\n \tadd_pending_object(rev, branch, name);\n \tadd_pending_object(rev, &head->object, \"^HEAD\");\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 430d876..8be9a28 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -68,8 +68,8 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tif (default_date_mode)\n \t\trev->date_mode = parse_date_format(default_date_mode);\n \n-\targc = setup_revisions(argc, argv, rev, \"HEAD\");\n-\n+\targc = parse_revisions(argc, argv, rev);\n+\tsetup_revisions(rev, \"HEAD\");\n \tif (rev->diffopt.pickaxe || rev->diffopt.filter)\n \t\trev->always_show_header = 0;\n \tif (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {\n@@ -919,7 +919,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (numbered_files && use_stdout)\n \t\tdie (\"--numbered-files and --stdout are mutually exclusive.\");\n \n-\targc = setup_revisions(argc, argv, &rev, \"HEAD\");\n+\targc = parse_revisions(argc, argv, &rev);\n+\tsetup_revisions(&rev, \"HEAD\");\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 2dadec1..3d43050 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1964,7 +1964,8 @@ static void get_object_list(int ac, const char **av)\n \n \tinit_revisions(&revs, NULL);\n \tsave_commit_buffer = 0;\n-\tsetup_revisions(ac, av, &revs, NULL);\n+\tparse_revisions(ac, av, &revs);\n+\tsetup_revisions(&revs, NULL);\n \n \twhile (fgets(line, sizeof(line), stdin) != NULL) {\n \t\tint len = strlen(line);\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex b4a2c44..a9f5e41 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -588,7 +588,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&revs, prefix);\n \trevs.abbrev = 0;\n \trevs.commit_format = CMIT_FMT_UNSPECIFIED;\n-\targc = setup_revisions(argc, argv, &revs, NULL);\n+\targc = parse_revisions(argc, argv, &revs);\n+\tsetup_revisions(&revs, NULL);\n \n \tfor (i = 1 ; i < argc; i++) {\n \t\tconst char *arg = argv[i];\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 0270f9b..0adff14 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -253,7 +253,7 @@ static int index_is_dirty(void)\n {\n \tstruct rev_info rev;\n \tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, \"HEAD\");\n+\tsetup_revisions(&rev, \"HEAD\");\n \tDIFF_OPT_SET(&rev.diffopt, QUIET);\n \tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n \trun_diff_index(&rev, 1);\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex e6a2865..3428bf6 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -256,7 +256,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\targc--;\n \t}\n \tinit_revisions(&rev, prefix);\n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\targc = parse_revisions(argc, argv, &rev);\n+\tsetup_revisions(&rev, NULL);\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \ndiff --git a/bundle.c b/bundle.c\nindex 0ba5df1..c4848d7 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -120,7 +120,8 @@ int verify_bundle(struct bundle_header *header, int verbose)\n \tif (revs.pending.nr != p->nr)\n \t\treturn ret;\n \treq_nr = revs.pending.nr;\n-\tsetup_revisions(2, argv, &revs, NULL);\n+\tparse_revisions(2, argv, &revs);\n+\tsetup_revisions(&revs, NULL);\n \n \tmemset(&refs, 0, sizeof(struct object_array));\n \tfor (i = 0; i < revs.pending.nr; i++) {\n@@ -226,7 +227,8 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn error(\"rev-list died\");\n \n \t/* write references */\n-\targc = setup_revisions(argc, argv, &revs, NULL);\n+\targc = parse_revisions(argc, argv, &revs);\n+\tsetup_revisions(&revs, NULL);\n \tif (argc > 1)\n \t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n \ndiff --git a/http-push.c b/http-push.c\nindex 2cd068a..aa348aa 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -2383,7 +2383,8 @@ int main(int argc, char **argv)\n \t\t\tcommit_argc++;\n \t\t}\n \t\tinit_revisions(&revs, setup_git_directory());\n-\t\tsetup_revisions(commit_argc, commit_argv, &revs, NULL);\n+\t\tparse_revisions(commit_argc, commit_argv, &revs);\n+\t\tsetup_revisions(&revs, NULL);\n \t\trevs.edge_hint = 0; /* just in case */\n \t\tfree(new_sha1_hex);\n \t\tif (old_sha1_hex) {\ndiff --git a/remote.c b/remote.c\nindex df8bd72..c95a510 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1280,7 +1280,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)\n \tstrcpy(symmetric + 43, sha1_to_hex(theirs->object.sha1));\n \n \tinit_revisions(&revs, NULL);\n-\tsetup_revisions(rev_argc, rev_argv, &revs, NULL);\n+\tparse_revisions(rev_argc, rev_argv, &revs);\n+\tsetup_revisions(&revs, NULL);\n \tprepare_revision_walk(&revs);\n \n \t/* ... and count the commits on each side. */\ndiff --git a/revision.c b/revision.c\nindex 0191160..3a46ed4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -981,14 +981,11 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)\n  * Returns the number of arguments left that weren't recognized\n  * (which are also moved to the head of the argument list)\n  */\n-int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)\n+int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n {\n-\tint i, flags, seen_dashdash, show_merge;\n+\tint i, flags, seen_dashdash;\n \tconst char **unrecognized = argv + 1;\n \tint left = 1;\n-\tint all_match = 0;\n-\tint regflags = 0;\n-\tint fixed = 0;\n \n \t/* First, search for \"--\" */\n \tseen_dashdash = 0;\n@@ -1004,7 +1001,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\tbreak;\n \t}\n \n-\tflags = show_merge = 0;\n+\tflags = 0;\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (*arg == '-') {\n@@ -1092,11 +1089,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\tif (!strcmp(arg, \"--default\")) {\n \t\t\t\tif (++i >= argc)\n \t\t\t\t\tdie(\"bad --default argument\");\n-\t\t\t\tdef = argv[i];\n+\t\t\t\trevs->def = argv[i];\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--merge\")) {\n-\t\t\t\tshow_merge = 1;\n+\t\t\t\trevs->show_merge = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--topo-order\")) {\n@@ -1302,21 +1299,25 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--extended-regexp\") ||\n \t\t\t    !strcmp(arg, \"-E\")) {\n-\t\t\t\tregflags |= REG_EXTENDED;\n+\t\t\t\tif (revs->grep_filter)\n+\t\t\t\t\trevs->grep_filter->regflags |= REG_EXTENDED;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--regexp-ignore-case\") ||\n \t\t\t    !strcmp(arg, \"-i\")) {\n-\t\t\t\tregflags |= REG_ICASE;\n+\t\t\t\tif (revs->grep_filter)\n+\t\t\t\t\trevs->grep_filter->regflags |= REG_ICASE;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--fixed-strings\") ||\n \t\t\t    !strcmp(arg, \"-F\")) {\n-\t\t\t\tfixed = 1;\n+\t\t\t\tif (revs->grep_filter)\n+\t\t\t\t\trevs->grep_filter->fixed = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--all-match\")) {\n-\t\t\t\tall_match = 1;\n+\t\t\t\tif (revs->grep_filter)\n+\t\t\t\t\trevs->grep_filter->all_match = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!prefixcmp(arg, \"--encoding=\")) {\n@@ -1374,22 +1375,23 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\tbreak;\n \t\t}\n \t}\n+\treturn left;\n+}\n \n-\tif (revs->grep_filter) {\n-\t\trevs->grep_filter->regflags |= regflags;\n-\t\trevs->grep_filter->fixed = fixed;\n-\t}\n-\n-\tif (show_merge)\n+void setup_revisions(struct rev_info *revs, const char *def)\n+{\n+\tif (revs->def == NULL)\n+\t\trevs->def = def;\n+\tif (revs->show_merge)\n \t\tprepare_show_merge(revs);\n-\tif (def && !revs->pending.nr) {\n+\tif (revs->def && !revs->pending.nr) {\n \t\tunsigned char sha1[20];\n \t\tstruct object *object;\n \t\tunsigned mode;\n-\t\tif (get_sha1_with_mode(def, sha1, &mode))\n-\t\t\tdie(\"bad default revision '%s'\", def);\n-\t\tobject = get_reference(revs, def, sha1, 0);\n-\t\tadd_pending_object_with_mode(revs, object, def, mode);\n+\t\tif (get_sha1_with_mode(revs->def, sha1, &mode))\n+\t\t\tdie(\"bad default revision '%s'\", revs->def);\n+\t\tobject = get_reference(revs, revs->def, sha1, 0);\n+\t\tadd_pending_object_with_mode(revs, object, revs->def, mode);\n \t}\n \n \t/* Did the user ask for any diff output? Run the diff! */\n@@ -1423,7 +1425,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\tdie(\"diff_setup_done failed\");\n \n \tif (revs->grep_filter) {\n-\t\trevs->grep_filter->all_match = all_match;\n \t\tcompile_grep_patterns(revs->grep_filter);\n \t}\n \n@@ -1440,8 +1441,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \n \tif (revs->reflog_info && revs->graph)\n \t\tdie(\"cannot combine --walk-reflogs with --graph\");\n-\n-\treturn left;\n }\n \n static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)\ndiff --git a/revision.h b/revision.h\nindex 5b8c56b..e45b86e 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -26,6 +26,7 @@ struct rev_info {\n \n \t/* Basic information */\n \tconst char *prefix;\n+\tconst char *def;\n \tvoid *prune_data;\n \tunsigned int early_output;\n \n@@ -66,8 +67,9 @@ struct rev_info {\n \n \t/* Format info */\n \tunsigned int\tshown_one:1,\n-\t\t\tabbrev_commit:1,\n+\t\t\tshow_merge:1,\n \t\t\tuse_terminator:1,\n+\t\t\tabbrev_commit:1,\n \t\t\tmissing_newline:1;\n \tenum date_mode date_mode;\n \n@@ -119,7 +121,8 @@ typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);\n volatile show_early_output_fn_t show_early_output;\n \n extern void init_revisions(struct rev_info *revs, const char *prefix);\n-extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);\n+extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);\n+extern void setup_revisions(struct rev_info *revs, const char *def);\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);\n \n extern int prepare_revision_walk(struct rev_info *revs);\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 9f82941..a87a9a3 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -116,7 +116,7 @@ static int do_rev_list(int fd, void *create_full_pack)\n \n \tif (create_full_pack) {\n \t\tconst char *args[] = {\"rev-list\", \"--all\", NULL};\n-\t\tsetup_revisions(2, args, &revs, NULL);\n+\t\tparse_revisions(2, args, &revs);\n \t} else {\n \t\tfor (i = 0; i < want_obj.nr; i++) {\n \t\t\tstruct object *o = want_obj.objects[i].item;\n@@ -129,8 +129,8 @@ static int do_rev_list(int fd, void *create_full_pack)\n \t\t\to->flags |= UNINTERESTING;\n \t\t\tadd_pending_object(&revs, o, NULL);\n \t\t}\n-\t\tsetup_revisions(0, NULL, &revs, NULL);\n \t}\n+\tsetup_revisions(&revs, NULL);\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \tmark_edges_uninteresting(revs.commits, &revs, show_edge);\ndiff --git a/wt-status.c b/wt-status.c\nindex 889e50f..df1130c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -206,7 +206,7 @@ static void wt_status_print_updated(struct wt_status *s)\n {\n \tstruct rev_info rev;\n \tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, s->reference);\n+\tsetup_revisions(&rev, s->reference);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_print_updated_cb;\n \trev.diffopt.format_callback_data = s;\n@@ -220,7 +220,7 @@ static void wt_status_print_changed(struct wt_status *s)\n {\n \tstruct rev_info rev;\n \tinit_revisions(&rev, \"\");\n-\tsetup_revisions(0, NULL, &rev, NULL);\n+\tsetup_revisions(&rev, NULL);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_print_changed_cb;\n \trev.diffopt.format_callback_data = s;\n@@ -308,7 +308,7 @@ static void wt_status_print_verbose(struct wt_status *s)\n \tstruct rev_info rev;\n \n \tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, s->reference);\n+\tsetup_revisions(&rev, s->reference);\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \trev.diffopt.detect_rename = 1;\n \trev.diffopt.file = s->fp;\n-- \n1.5.6.2.352.g416a6\n"},{"id":"82577","messageId":"1215510964-16664-3-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":"1215510964-16664-2-git-send-email-madcoder@debian.org","subject":"[PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:56:00Z","receivedAt":"2008-07-08T09:56:00Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n revision.c |  579 ++++++++++++++++++++++++++++--------------------------------\n revision.h |    2 +\n 2 files changed, 273 insertions(+), 308 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 3a46ed4..a3792c9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -974,6 +974,273 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)\n \trevs->ignore_packed[num] = NULL;\n }\n \n+int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n+\t\t\tint *unkc, const char **unkv)\n+{\n+\tconst char *arg = argv[0];\n+\n+\t/* pseudo revision arguments */\n+\tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n+\t    !strcmp(arg, \"--tags\") || !strcmp(arg, \"--remotes\") ||\n+\t    !strcmp(arg, \"--reflog\") || !strcmp(arg, \"--not\") ||\n+\t    !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\"))\n+\t{\n+\t\tunkv[*unkc++] = arg;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!prefixcmp(arg, \"--max-count=\")) {\n+\t\trevs->max_count = atoi(arg + 12);\n+\t}\n+\telse if (!prefixcmp(arg, \"--skip=\")) {\n+\t\trevs->skip_count = atoi(arg + 7);\n+\t}\n+\t/* accept -<digit>, like traditional \"head\" */\n+\telse if ((*arg == '-') && isdigit(arg[1])) {\n+\t\trevs->max_count = atoi(arg + 1);\n+\t}\n+\telse if (!strcmp(arg, \"-n\")) {\n+\t\tif (argc <= 1)\n+\t\t\treturn error(\"-n requires an argument\");\n+\t\trevs->max_count = atoi(argv[1]);\n+\t\treturn 2;\n+\t}\n+\telse if (!prefixcmp(arg, \"-n\")) {\n+\t\trevs->max_count = atoi(arg + 2);\n+\t}\n+\telse if (!prefixcmp(arg, \"--max-age=\")) {\n+\t\trevs->max_age = atoi(arg + 10);\n+\t}\n+\telse if (!prefixcmp(arg, \"--since=\")) {\n+\t\trevs->max_age = approxidate(arg + 8);\n+\t}\n+\telse if (!prefixcmp(arg, \"--after=\")) {\n+\t\trevs->max_age = approxidate(arg + 8);\n+\t}\n+\telse if (!prefixcmp(arg, \"--min-age=\")) {\n+\t\trevs->min_age = atoi(arg + 10);\n+\t}\n+\telse if (!prefixcmp(arg, \"--before=\")) {\n+\t\trevs->min_age = approxidate(arg + 9);\n+\t}\n+\telse if (!prefixcmp(arg, \"--until=\")) {\n+\t\trevs->min_age = approxidate(arg + 8);\n+\t}\n+\telse if (!strcmp(arg, \"--first-parent\")) {\n+\t\trevs->first_parent_only = 1;\n+\t}\n+\telse if (!strcmp(arg, \"-g\") ||\n+\t    !strcmp(arg, \"--walk-reflogs\")) {\n+\t\tinit_reflog_walk(&revs->reflog_info);\n+\t}\n+\telse if (!strcmp(arg, \"--default\")) {\n+\t\tif (argc <= 1)\n+\t\t\treturn error(\"bad --default argument\");\n+\t\trevs->def = argv[1];\n+\t\treturn 2;\n+\t}\n+\telse if (!strcmp(arg, \"--merge\")) {\n+\t\trevs->show_merge = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--topo-order\")) {\n+\t\trevs->lifo = 1;\n+\t\trevs->topo_order = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--date-order\")) {\n+\t\trevs->lifo = 0;\n+\t\trevs->topo_order = 1;\n+\t}\n+\telse if (!prefixcmp(arg, \"--early-output\")) {\n+\t\tint count = 100;\n+\t\tswitch (arg[14]) {\n+\t\t  case '=':\n+\t\t    count = atoi(arg+15);\n+\t\t    /* Fallthrough */\n+\t\t  case 0:\n+\t\t    revs->topo_order = 1;\n+\t\t    revs->early_output = count;\n+\t\t}\n+\t}\n+\telse if (!strcmp(arg, \"--parents\")) {\n+\t\trevs->rewrite_parents = 1;\n+\t\trevs->print_parents = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--dense\")) {\n+\t\trevs->dense = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--sparse\")) {\n+\t\trevs->dense = 0;\n+\t}\n+\telse if (!strcmp(arg, \"--show-all\")) {\n+\t\trevs->show_all = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--remove-empty\")) {\n+\t\trevs->remove_empty_trees = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--no-merges\")) {\n+\t\trevs->no_merges = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--boundary\")) {\n+\t\trevs->boundary = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--left-right\")) {\n+\t\trevs->left_right = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--cherry-pick\")) {\n+\t\trevs->cherry_pick = 1;\n+\t\trevs->limited = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--objects\")) {\n+\t\trevs->tag_objects = 1;\n+\t\trevs->tree_objects = 1;\n+\t\trevs->blob_objects = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--objects-edge\")) {\n+\t\trevs->tag_objects = 1;\n+\t\trevs->tree_objects = 1;\n+\t\trevs->blob_objects = 1;\n+\t\trevs->edge_hint = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--unpacked\")) {\n+\t\trevs->unpacked = 1;\n+\t\tfree(revs->ignore_packed);\n+\t\trevs->ignore_packed = NULL;\n+\t\trevs->num_ignore_packed = 0;\n+\t}\n+\telse if (!prefixcmp(arg, \"--unpacked=\")) {\n+\t\trevs->unpacked = 1;\n+\t\tadd_ignore_packed(revs, arg+11);\n+\t}\n+\telse if (!strcmp(arg, \"-r\")) {\n+\t\trevs->diff = 1;\n+\t\tDIFF_OPT_SET(&revs->diffopt, RECURSIVE);\n+\t}\n+\telse if (!strcmp(arg, \"-t\")) {\n+\t\trevs->diff = 1;\n+\t\tDIFF_OPT_SET(&revs->diffopt, RECURSIVE);\n+\t\tDIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);\n+\t}\n+\telse if (!strcmp(arg, \"-m\")) {\n+\t\trevs->ignore_merges = 0;\n+\t}\n+\telse if (!strcmp(arg, \"-c\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->dense_combined_merges = 0;\n+\t\trevs->combine_merges = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--cc\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->dense_combined_merges = 1;\n+\t\trevs->combine_merges = 1;\n+\t}\n+\telse if (!strcmp(arg, \"-v\")) {\n+\t\trevs->verbose_header = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--pretty\")) {\n+\t\trevs->verbose_header = 1;\n+\t\tget_commit_format(arg+8, revs);\n+\t}\n+\telse if (!prefixcmp(arg, \"--pretty=\")) {\n+\t\trevs->verbose_header = 1;\n+\t\tget_commit_format(arg+9, revs);\n+\t}\n+\telse if (!strcmp(arg, \"--graph\")) {\n+\t\trevs->topo_order = 1;\n+\t\trevs->rewrite_parents = 1;\n+\t\trevs->graph = graph_init(revs);\n+\t}\n+\telse if (!strcmp(arg, \"--root\")) {\n+\t\trevs->show_root_diff = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--no-commit-id\")) {\n+\t\trevs->no_commit_id = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--always\")) {\n+\t\trevs->always_show_header = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--no-abbrev\")) {\n+\t\trevs->abbrev = 0;\n+\t}\n+\telse if (!strcmp(arg, \"--abbrev\")) {\n+\t\trevs->abbrev = DEFAULT_ABBREV;\n+\t}\n+\telse if (!prefixcmp(arg, \"--abbrev=\")) {\n+\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n+\t\tif (revs->abbrev < MINIMUM_ABBREV)\n+\t\t\trevs->abbrev = MINIMUM_ABBREV;\n+\t\telse if (revs->abbrev > 40)\n+\t\t\trevs->abbrev = 40;\n+\t}\n+\telse if (!strcmp(arg, \"--abbrev-commit\")) {\n+\t\trevs->abbrev_commit = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--full-diff\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->full_diff = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--full-history\")) {\n+\t\trevs->simplify_history = 0;\n+\t}\n+\telse if (!strcmp(arg, \"--relative-date\")) {\n+\t\trevs->date_mode = DATE_RELATIVE;\n+\t}\n+\telse if (!strncmp(arg, \"--date=\", 7)) {\n+\t\trevs->date_mode = parse_date_format(arg + 7);\n+\t}\n+\telse if (!strcmp(arg, \"--log-size\")) {\n+\t\trevs->show_log_size = 1;\n+\t}\n+\t/*\n+\t * Grepping the commit log\n+\t */\n+\telse if (!prefixcmp(arg, \"--author=\")) {\n+\t\tadd_header_grep(revs, \"author\", arg+9);\n+\t}\n+\telse if (!prefixcmp(arg, \"--committer=\")) {\n+\t\tadd_header_grep(revs, \"committer\", arg+12);\n+\t}\n+\telse if (!prefixcmp(arg, \"--grep=\")) {\n+\t\tadd_message_grep(revs, arg+7);\n+\t}\n+\telse if (!strcmp(arg, \"--extended-regexp\") ||\n+\t    !strcmp(arg, \"-E\")) {\n+\t\tif (revs->grep_filter)\n+\t\t\trevs->grep_filter->regflags |= REG_EXTENDED;\n+\t}\n+\telse if (!strcmp(arg, \"--regexp-ignore-case\") ||\n+\t    !strcmp(arg, \"-i\")) {\n+\t\tif (revs->grep_filter)\n+\t\t\trevs->grep_filter->regflags |= REG_ICASE;\n+\t}\n+\telse if (!strcmp(arg, \"--fixed-strings\") ||\n+\t    !strcmp(arg, \"-F\")) {\n+\t\tif (revs->grep_filter)\n+\t\t\trevs->grep_filter->fixed = 1;\n+\t}\n+\telse if (!strcmp(arg, \"--all-match\")) {\n+\t\tif (revs->grep_filter)\n+\t\t\trevs->grep_filter->all_match = 1;\n+\t}\n+\telse if (!prefixcmp(arg, \"--encoding=\")) {\n+\t\targ += 11;\n+\t\tif (strcmp(arg, \"none\"))\n+\t\t\tgit_log_output_encoding = xstrdup(arg);\n+\t\telse\n+\t\t\tgit_log_output_encoding = \"\";\n+\t}\n+\telse if (!strcmp(arg, \"--reverse\")) {\n+\t\trevs->reverse ^= 1;\n+\t}\n+\telse if (!strcmp(arg, \"--children\")) {\n+\t\trevs->children.name = \"children\";\n+\t\trevs->limited = 1;\n+\t}\n+\telse\n+\t\treturn diff_opt_parse(&revs->diffopt, argv, argc);\n+\n+\treturn 1;\n+}\n+\n /*\n  * Parse revision information, filling in the \"rev_info\" structure,\n  * and removing the used arguments from the argument list.\n@@ -1006,53 +1273,7 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\tconst char *arg = argv[i];\n \t\tif (*arg == '-') {\n \t\t\tint opts;\n-\t\t\tif (!prefixcmp(arg, \"--max-count=\")) {\n-\t\t\t\trevs->max_count = atoi(arg + 12);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--skip=\")) {\n-\t\t\t\trevs->skip_count = atoi(arg + 7);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\t/* accept -<digit>, like traditional \"head\" */\n-\t\t\tif ((*arg == '-') && isdigit(arg[1])) {\n-\t\t\t\trevs->max_count = atoi(arg + 1);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"-n\")) {\n-\t\t\t\tif (argc <= i + 1)\n-\t\t\t\t\tdie(\"-n requires an argument\");\n-\t\t\t\trevs->max_count = atoi(argv[++i]);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"-n\")) {\n-\t\t\t\trevs->max_count = atoi(arg + 2);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--max-age=\")) {\n-\t\t\t\trevs->max_age = atoi(arg + 10);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--since=\")) {\n-\t\t\t\trevs->max_age = approxidate(arg + 8);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--after=\")) {\n-\t\t\t\trevs->max_age = approxidate(arg + 8);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--min-age=\")) {\n-\t\t\t\trevs->min_age = atoi(arg + 10);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--before=\")) {\n-\t\t\t\trevs->min_age = approxidate(arg + 9);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--until=\")) {\n-\t\t\t\trevs->min_age = approxidate(arg + 8);\n-\t\t\t\tcontinue;\n-\t\t\t}\n+\n \t\t\tif (!strcmp(arg, \"--all\")) {\n \t\t\t\thandle_refs(revs, flags, for_each_ref);\n \t\t\t\tcontinue;\n@@ -1069,269 +1290,14 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\t\thandle_refs(revs, flags, for_each_remote_ref);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!strcmp(arg, \"--first-parent\")) {\n-\t\t\t\trevs->first_parent_only = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n \t\t\tif (!strcmp(arg, \"--reflog\")) {\n \t\t\t\thandle_reflog(revs, flags);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!strcmp(arg, \"-g\") ||\n-\t\t\t\t\t!strcmp(arg, \"--walk-reflogs\")) {\n-\t\t\t\tinit_reflog_walk(&revs->reflog_info);\n-\t\t\t\tcontinue;\n-\t\t\t}\n \t\t\tif (!strcmp(arg, \"--not\")) {\n \t\t\t\tflags ^= UNINTERESTING;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!strcmp(arg, \"--default\")) {\n-\t\t\t\tif (++i >= argc)\n-\t\t\t\t\tdie(\"bad --default argument\");\n-\t\t\t\trevs->def = argv[i];\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--merge\")) {\n-\t\t\t\trevs->show_merge = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--topo-order\")) {\n-\t\t\t\trevs->lifo = 1;\n-\t\t\t\trevs->topo_order = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--date-order\")) {\n-\t\t\t\trevs->lifo = 0;\n-\t\t\t\trevs->topo_order = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--early-output\")) {\n-\t\t\t\tint count = 100;\n-\t\t\t\tswitch (arg[14]) {\n-\t\t\t\tcase '=':\n-\t\t\t\t\tcount = atoi(arg+15);\n-\t\t\t\t\t/* Fallthrough */\n-\t\t\t\tcase 0:\n-\t\t\t\t\trevs->topo_order = 1;\n-\t\t\t\t\trevs->early_output = count;\n-\t\t\t\t\tcontinue;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--parents\")) {\n-\t\t\t\trevs->rewrite_parents = 1;\n-\t\t\t\trevs->print_parents = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--dense\")) {\n-\t\t\t\trevs->dense = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--sparse\")) {\n-\t\t\t\trevs->dense = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--show-all\")) {\n-\t\t\t\trevs->show_all = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--remove-empty\")) {\n-\t\t\t\trevs->remove_empty_trees = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--no-merges\")) {\n-\t\t\t\trevs->no_merges = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--boundary\")) {\n-\t\t\t\trevs->boundary = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--left-right\")) {\n-\t\t\t\trevs->left_right = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--cherry-pick\")) {\n-\t\t\t\trevs->cherry_pick = 1;\n-\t\t\t\trevs->limited = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--objects\")) {\n-\t\t\t\trevs->tag_objects = 1;\n-\t\t\t\trevs->tree_objects = 1;\n-\t\t\t\trevs->blob_objects = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--objects-edge\")) {\n-\t\t\t\trevs->tag_objects = 1;\n-\t\t\t\trevs->tree_objects = 1;\n-\t\t\t\trevs->blob_objects = 1;\n-\t\t\t\trevs->edge_hint = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--unpacked\")) {\n-\t\t\t\trevs->unpacked = 1;\n-\t\t\t\tfree(revs->ignore_packed);\n-\t\t\t\trevs->ignore_packed = NULL;\n-\t\t\t\trevs->num_ignore_packed = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--unpacked=\")) {\n-\t\t\t\trevs->unpacked = 1;\n-\t\t\t\tadd_ignore_packed(revs, arg+11);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"-r\")) {\n-\t\t\t\trevs->diff = 1;\n-\t\t\t\tDIFF_OPT_SET(&revs->diffopt, RECURSIVE);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"-t\")) {\n-\t\t\t\trevs->diff = 1;\n-\t\t\t\tDIFF_OPT_SET(&revs->diffopt, RECURSIVE);\n-\t\t\t\tDIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"-m\")) {\n-\t\t\t\trevs->ignore_merges = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"-c\")) {\n-\t\t\t\trevs->diff = 1;\n-\t\t\t\trevs->dense_combined_merges = 0;\n-\t\t\t\trevs->combine_merges = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--cc\")) {\n-\t\t\t\trevs->diff = 1;\n-\t\t\t\trevs->dense_combined_merges = 1;\n-\t\t\t\trevs->combine_merges = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"-v\")) {\n-\t\t\t\trevs->verbose_header = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--pretty\")) {\n-\t\t\t\trevs->verbose_header = 1;\n-\t\t\t\tget_commit_format(arg+8, revs);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--pretty=\")) {\n-\t\t\t\trevs->verbose_header = 1;\n-\t\t\t\tget_commit_format(arg+9, revs);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--graph\")) {\n-\t\t\t\trevs->topo_order = 1;\n-\t\t\t\trevs->rewrite_parents = 1;\n-\t\t\t\trevs->graph = graph_init(revs);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--root\")) {\n-\t\t\t\trevs->show_root_diff = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--no-commit-id\")) {\n-\t\t\t\trevs->no_commit_id = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--always\")) {\n-\t\t\t\trevs->always_show_header = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--no-abbrev\")) {\n-\t\t\t\trevs->abbrev = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--abbrev\")) {\n-\t\t\t\trevs->abbrev = DEFAULT_ABBREV;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--abbrev=\")) {\n-\t\t\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n-\t\t\t\tif (revs->abbrev < MINIMUM_ABBREV)\n-\t\t\t\t\trevs->abbrev = MINIMUM_ABBREV;\n-\t\t\t\telse if (revs->abbrev > 40)\n-\t\t\t\t\trevs->abbrev = 40;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--abbrev-commit\")) {\n-\t\t\t\trevs->abbrev_commit = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--full-diff\")) {\n-\t\t\t\trevs->diff = 1;\n-\t\t\t\trevs->full_diff = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--full-history\")) {\n-\t\t\t\trevs->simplify_history = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--relative-date\")) {\n-\t\t\t\trevs->date_mode = DATE_RELATIVE;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strncmp(arg, \"--date=\", 7)) {\n-\t\t\t\trevs->date_mode = parse_date_format(arg + 7);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--log-size\")) {\n-\t\t\t\trevs->show_log_size = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\n-\t\t\t/*\n-\t\t\t * Grepping the commit log\n-\t\t\t */\n-\t\t\tif (!prefixcmp(arg, \"--author=\")) {\n-\t\t\t\tadd_header_grep(revs, \"author\", arg+9);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--committer=\")) {\n-\t\t\t\tadd_header_grep(revs, \"committer\", arg+12);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--grep=\")) {\n-\t\t\t\tadd_message_grep(revs, arg+7);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--extended-regexp\") ||\n-\t\t\t    !strcmp(arg, \"-E\")) {\n-\t\t\t\tif (revs->grep_filter)\n-\t\t\t\t\trevs->grep_filter->regflags |= REG_EXTENDED;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--regexp-ignore-case\") ||\n-\t\t\t    !strcmp(arg, \"-i\")) {\n-\t\t\t\tif (revs->grep_filter)\n-\t\t\t\t\trevs->grep_filter->regflags |= REG_ICASE;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--fixed-strings\") ||\n-\t\t\t    !strcmp(arg, \"-F\")) {\n-\t\t\t\tif (revs->grep_filter)\n-\t\t\t\t\trevs->grep_filter->fixed = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--all-match\")) {\n-\t\t\t\tif (revs->grep_filter)\n-\t\t\t\t\trevs->grep_filter->all_match = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--encoding=\")) {\n-\t\t\t\targ += 11;\n-\t\t\t\tif (strcmp(arg, \"none\"))\n-\t\t\t\t\tgit_log_output_encoding = xstrdup(arg);\n-\t\t\t\telse\n-\t\t\t\t\tgit_log_output_encoding = \"\";\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--reverse\")) {\n-\t\t\t\trevs->reverse ^= 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n \t\t\tif (!strcmp(arg, \"--no-walk\")) {\n \t\t\t\trevs->no_walk = 1;\n \t\t\t\tcontinue;\n@@ -1340,17 +1306,14 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\t\trevs->no_walk = 0;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!strcmp(arg, \"--children\")) {\n-\t\t\t\trevs->children.name = \"children\";\n-\t\t\t\trevs->limited = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n \n-\t\t\topts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);\n+\t\t\topts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);\n \t\t\tif (opts > 0) {\n \t\t\t\ti += opts - 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (opts < 0)\n+\t\t\t\texit(128);\n \t\t\t*unrecognized++ = arg;\n \t\t\tleft++;\n \t\t\tcontinue;\ndiff --git a/revision.h b/revision.h\nindex e45b86e..46ab713 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -123,6 +123,8 @@ volatile show_early_output_fn_t show_early_output;\n extern void init_revisions(struct rev_info *revs, const char *prefix);\n extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);\n extern void setup_revisions(struct rev_info *revs, const char *def);\n+extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n+\t\t\t       int *unkc, const char **unkv);\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);\n \n extern int prepare_revision_walk(struct rev_info *revs);\n-- \n1.5.6.2.352.g416a6\n"},{"id":"82576","messageId":"1215510964-16664-4-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":"1215510964-16664-3-git-send-email-madcoder@debian.org","subject":"[PATCH 3/6] revisions: parse_revisions refactor.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:56:01Z","receivedAt":"2008-07-08T09:56:01Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Reorder parse_revisions so that it works more like a parse-opt based\nparser would: use two pass, the first filtering out options that the\nrevision parser understands, and remember if we saw a \"--\", and then\nparse revision arguments.\n\nOnce everyone is using a parse-opt parser, it will be possible to simplify\nparse_revisions further. The first pass is a NOOP for incremental\nparse-opt based parsers.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n revision.c |   63 ++++++++++++++++++++++++++++++++++++-----------------------\n 1 files changed, 38 insertions(+), 25 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex a3792c9..87f89a2 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1250,30 +1250,44 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n  */\n int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n {\n-\tint i, flags, seen_dashdash;\n-\tconst char **unrecognized = argv + 1;\n-\tint left = 1;\n+\tint i, left, flags;\n+\tint seen_dashdash = 0;\n \n-\t/* First, search for \"--\" */\n-\tseen_dashdash = 0;\n-\tfor (i = 1; i < argc; i++) {\n+\t/* First, filter out revision options and look for \"--\" */\n+\tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-\t\tif (strcmp(arg, \"--\"))\n+\t\tint opts;\n+\n+\t\tif (arg[0] != '-') {\n+\t\t\targv[left++] = arg;\n \t\t\tcontinue;\n-\t\targv[i] = NULL;\n-\t\targc = i;\n-\t\tif (argv[i + 1])\n-\t\t\trevs->prune_data = get_pathspec(revs->prefix, argv + i + 1);\n-\t\tseen_dashdash = 1;\n-\t\tbreak;\n+\t\t}\n+\n+\t\tif (!strcmp(arg, \"--\")) {\n+\t\t\tseen_dashdash = left;\n+\t\t\tmemcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));\n+\t\t\tleft += argc - i;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n+\t\tif (opts > 0) {\n+\t\t\ti += opts - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (opts < 0)\n+\t\t\texit(128);\n+\t\targv[left++] = arg;\n \t}\n+\targv[left] = NULL;\n+\targc = left;\n \n+\t/* Second, parse revisions arguments */\n \tflags = 0;\n-\tfor (i = 1; i < argc; i++) {\n+\tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-\t\tif (*arg == '-') {\n-\t\t\tint opts;\n \n+\t\tif (*arg == '-') {\n \t\t\tif (!strcmp(arg, \"--all\")) {\n \t\t\t\thandle_refs(revs, flags, for_each_ref);\n \t\t\t\tcontinue;\n@@ -1306,16 +1320,15 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\t\trevs->no_walk = 0;\n \t\t\t\tcontinue;\n \t\t\t}\n-\n-\t\t\topts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);\n-\t\t\tif (opts > 0) {\n-\t\t\t\ti += opts - 1;\n-\t\t\t\tcontinue;\n+\t\t\tif (i == seen_dashdash) {\n+\t\t\t\targv[i] = NULL;\n+\t\t\t\targc = i;\n+\t\t\t\tif (argv[i + 1])\n+\t\t\t\t\trevs->prune_data = get_pathspec(revs->prefix, argv + i + 1);\n+\t\t\t\tbreak;\n \t\t\t}\n-\t\t\tif (opts < 0)\n-\t\t\t\texit(128);\n-\t\t\t*unrecognized++ = arg;\n-\t\t\tleft++;\n+\n+\t\t\targv[left++] = arg;\n \t\t\tcontinue;\n \t\t}\n \n-- \n1.5.6.2.352.g416a6\n"},{"id":"82574","messageId":"1215510964-16664-5-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":"1215510964-16664-4-git-send-email-madcoder@debian.org","subject":"[PATCH 4/6] revisions: split handle_revision_args from parse_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:56:02Z","receivedAt":"2008-07-08T09:56:02Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"This new function is meant to only parse non option revision arguments in\na row. IOW it's meant to parse what remains from a parse-opt filtering of\nthe argument list, with the knowledge of the \"--\" position (0 means none).\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n revision.c |   97 +++++++++++++++++++++++++++++++----------------------------\n revision.h |    1 +\n 2 files changed, 52 insertions(+), 46 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 87f89a2..c7de30f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1241,49 +1241,11 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n \treturn 1;\n }\n \n-/*\n- * Parse revision information, filling in the \"rev_info\" structure,\n- * and removing the used arguments from the argument list.\n- *\n- * Returns the number of arguments left that weren't recognized\n- * (which are also moved to the head of the argument list)\n- */\n-int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n+int handle_revision_args(struct rev_info *revs,\n+                         int argc, const char **argv, int dashdash_pos)\n {\n-\tint i, left, flags;\n-\tint seen_dashdash = 0;\n-\n-\t/* First, filter out revision options and look for \"--\" */\n-\tfor (left = i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tint opts;\n-\n-\t\tif (arg[0] != '-') {\n-\t\t\targv[left++] = arg;\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (!strcmp(arg, \"--\")) {\n-\t\t\tseen_dashdash = left;\n-\t\t\tmemcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));\n-\t\t\tleft += argc - i;\n-\t\t\tbreak;\n-\t\t}\n+\tint i, left, flags = 0;\n \n-\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n-\t\tif (opts > 0) {\n-\t\t\ti += opts - 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (opts < 0)\n-\t\t\texit(128);\n-\t\targv[left++] = arg;\n-\t}\n-\targv[left] = NULL;\n-\targc = left;\n-\n-\t/* Second, parse revisions arguments */\n-\tflags = 0;\n \tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \n@@ -1320,7 +1282,7 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\t\trevs->no_walk = 0;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (i == seen_dashdash) {\n+\t\t\tif (i == dashdash_pos) {\n \t\t\t\targv[i] = NULL;\n \t\t\t\targc = i;\n \t\t\t\tif (argv[i + 1])\n@@ -1332,10 +1294,10 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (handle_revision_arg(arg, revs, flags, seen_dashdash)) {\n+\t\tif (handle_revision_arg(arg, revs, flags, dashdash_pos)) {\n \t\t\tint j;\n-\t\t\tif (seen_dashdash || *arg == '^')\n-\t\t\t\tdie(\"bad revision '%s'\", arg);\n+\t\t\tif (dashdash_pos || *arg == '^')\n+\t\t\t\treturn error(\"bad revision '%s'\", arg);\n \n \t\t\t/* If we didn't have a \"--\":\n \t\t\t * (1) all filenames must exist;\n@@ -1351,7 +1313,50 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\tbreak;\n \t\t}\n \t}\n-\treturn left;\n+\n+        return left;\n+}\n+\n+/*\n+ * Parse revision information, filling in the \"rev_info\" structure,\n+ * and removing the used arguments from the argument list.\n+ *\n+ * Returns the number of arguments left that weren't recognized\n+ * (which are also moved to the head of the argument list)\n+ */\n+int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n+{\n+\tint i, left, dashdash_pos = 0;\n+\n+\t/* First, filter out revision options and look for \"--\" */\n+\tfor (left = i = 1; i < argc; i++) {\n+\t\tconst char *arg = argv[i];\n+\t\tint opts;\n+\n+\t\tif (arg[0] != '-') {\n+\t\t\targv[left++] = arg;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!strcmp(arg, \"--\")) {\n+\t\t\tdashdash_pos = left;\n+\t\t\tmemcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));\n+\t\t\tleft += argc - i;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n+\t\tif (opts > 0) {\n+\t\t\ti += opts - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (opts < 0)\n+\t\t\texit(128);\n+\t\targv[left++] = arg;\n+\t}\n+\targv[left] = NULL;\n+\n+        return handle_revision_args(revs, left, argv, dashdash_pos);\n }\n \n void setup_revisions(struct rev_info *revs, const char *def)\ndiff --git a/revision.h b/revision.h\nindex 46ab713..c0f5df0 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -125,6 +125,7 @@ extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);\n extern void setup_revisions(struct rev_info *revs, const char *def);\n extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n \t\t\t       int *unkc, const char **unkv);\n+extern int handle_revision_args(struct rev_info *revs, int argc, const char **argv, int dashdash_pos);\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);\n \n extern int prepare_revision_walk(struct rev_info *revs);\n-- \n1.5.6.2.352.g416a6\n"},{"id":"82573","messageId":"1215510964-16664-6-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":"1215510964-16664-5-git-send-email-madcoder@debian.org","subject":"[PATCH 5/6] git-blame: migrate to incremental parse-option [1/2]","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:56:03Z","receivedAt":"2008-07-08T09:56:03Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"This step merely moves the parser to an incremental version, still using\nparse_revisions.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n builtin-blame.c |  220 +++++++++++++++++++++++++++++-------------------------\n 1 files changed, 118 insertions(+), 102 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 1e26b88..6256341 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -18,24 +18,16 @@\n #include \"cache-tree.h\"\n #include \"path-list.h\"\n #include \"mailmap.h\"\n+#include \"parse-options.h\"\n \n-static char blame_usage[] =\n-\"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\"\n-\"  -c                  Use the same output mode as git-annotate (Default: off)\\n\"\n-\"  -b                  Show blank SHA-1 for boundary commits (Default: off)\\n\"\n-\"  -l                  Show long commit SHA1 (Default: off)\\n\"\n-\"  --root              Do not treat root commits as boundaries (Default: off)\\n\"\n-\"  -t                  Show raw timestamp (Default: off)\\n\"\n-\"  -f, --show-name     Show original filename (Default: auto)\\n\"\n-\"  -n, --show-number   Show original linenumber (Default: off)\\n\"\n-\"  -s                  Suppress author name and timestamp (Default: off)\\n\"\n-\"  -p, --porcelain     Show in a format designed for machine consumption\\n\"\n-\"  -w                  Ignore whitespace differences\\n\"\n-\"  -L n,m              Process only line range n,m, counting from 1\\n\"\n-\"  -M, -C              Find line movements within and across files\\n\"\n-\"  --incremental       Show blame entries as we find them, incrementally\\n\"\n-\"  --contents file     Use <file>'s contents as the final image\\n\"\n-\"  -S revs-file        Use revisions from revs-file instead of calling git-rev-list\\n\";\n+static char blame_usage[] = \"git-blame [options] [rev-opts] [rev] [--] file\";\n+\n+static const char *blame_opt_usage[] = {\n+\tblame_usage,\n+\t\"\",\n+\t\"[rev-opts] are documented in git-rev-parse(1)\",\n+\tNULL\n+};\n \n static int longest_file;\n static int longest_author;\n@@ -2219,6 +2211,50 @@ static const char *prepare_initial(struct scoreboard *sb)\n \treturn final_commit_name;\n }\n \n+static int blame_copy_callback(const struct option *option, const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\t/*\n+\t * -C enables copy from removed files;\n+\t * -C -C enables copy from existing files, but only\n+\t *       when blaming a new file;\n+\t * -C -C -C enables copy from existing files for\n+\t *          everybody\n+\t */\n+\tif (*opt & PICKAXE_BLAME_COPY_HARDER)\n+\t\t*opt |= PICKAXE_BLAME_COPY_HARDEST;\n+\tif (*opt & PICKAXE_BLAME_COPY)\n+\t\t*opt |= PICKAXE_BLAME_COPY_HARDER;\n+\t*opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;\n+\n+\tif (arg)\n+\t\tblame_copy_score = parse_score(arg);\n+\treturn 0;\n+}\n+\n+static int blame_move_callback(const struct option *option, const char *arg, int unset)\n+{\n+\tint *opt = option->value;\n+\n+\t*opt |= PICKAXE_BLAME_MOVE;\n+\n+\tif (arg)\n+\t\tblame_move_score = parse_score(arg);\n+\treturn 0;\n+}\n+\n+static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)\n+{\n+\tconst char **bottomtop = option->value;\n+\tif (!arg)\n+\t\treturn -1;\n+\tif (*bottomtop)\n+\t\tdie(\"More than one '-L n,m' option given\");\n+\t*bottomtop = arg;\n+\treturn 0;\n+}\n+\n int cmd_blame(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -2226,98 +2262,79 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tstruct scoreboard sb;\n \tstruct origin *o;\n \tstruct blame_entry *ent;\n-\tint i, seen_dashdash, unk, opt;\n+\tint i, seen_dashdash, unk;\n \tlong bottom, top, lno;\n-\tint output_option = 0;\n-\tint show_stats = 0;\n-\tconst char *revs_file = NULL;\n \tconst char *final_commit_name = NULL;\n \tenum object_type type;\n-\tconst char *bottomtop = NULL;\n-\tconst char *contents_from = NULL;\n+\n+\tstatic const char *bottomtop = NULL;\n+\tstatic int output_option = 0, opt = 0;\n+\tstatic int show_stats = 0;\n+\tstatic const char *revs_file = NULL;\n+\tstatic const char *contents_from = NULL;\n+\tstatic const struct option options[] = {\n+\t\tOPT_BOOLEAN(0, \"incremental\", &incremental, \"Show blame entries as we find them, incrementally\"),\n+\t\tOPT_BOOLEAN('b', NULL, &blank_boundary, \"Show blank SHA-1 for boundary commits (Default: off)\"),\n+\t\tOPT_BOOLEAN(0, \"root\", &show_root, \"Do not treat root commits as boundaries (Default: off)\"),\n+\t\tOPT_BOOLEAN(0, \"show-stats\", &show_stats, \"Show work cost statistics\"),\n+\t\tOPT_BIT(0, \"score-debug\", &output_option, \"Show output score for blame entries\", OUTPUT_SHOW_SCORE),\n+\t\tOPT_BIT('f', \"show-name\", &output_option, \"Show original filename (Default: auto)\", OUTPUT_SHOW_NAME),\n+\t\tOPT_BIT('n', \"show-number\", &output_option, \"Show original linenumber (Default: off)\", OUTPUT_SHOW_NUMBER),\n+\t\tOPT_BIT('p', \"porcelain\", &output_option, \"Show in a format designed for machine consumption\", OUTPUT_PORCELAIN),\n+\t\tOPT_BIT('c', NULL, &output_option, \"Use the same output mode as git-annotate (Default: off)\", OUTPUT_ANNOTATE_COMPAT),\n+\t\tOPT_BIT('t', NULL, &output_option, \"Show raw timestamp (Default: off)\", OUTPUT_RAW_TIMESTAMP),\n+\t\tOPT_BIT('l', NULL, &output_option, \"Show long commit SHA1 (Default: off)\", OUTPUT_LONG_OBJECT_NAME),\n+\t\tOPT_BIT('s', NULL, &output_option, \"Suppress author name and timestamp (Default: off)\", OUTPUT_NO_AUTHOR),\n+\t\tOPT_BIT('w', NULL, &xdl_opts, \"Ignore whitespace differences\", XDF_IGNORE_WHITESPACE),\n+\t\tOPT_STRING('S', NULL, &revs_file, \"file\", \"Use revisions from <file> instead of calling git-rev-list\"),\n+\t\tOPT_STRING(0, \"contents\", &contents_from, \"file\", \"Use <file>'s contents as the final image\"),\n+\t\t{ OPTION_CALLBACK, 'C', NULL, &opt, \"score\", \"Find line copies within and across files\", PARSE_OPT_OPTARG, blame_copy_callback },\n+\t\t{ OPTION_CALLBACK, 'M', NULL, &opt, \"score\", \"Find line movements within and across files\", PARSE_OPT_OPTARG, blame_move_callback },\n+\t\tOPT_CALLBACK('L', NULL, &bottomtop, \"n,m\", \"Process only line range n,m, counting from 1\", blame_bottomtop_callback),\n+\t\tOPT_END()\n+\t};\n+\n+\tstruct parse_opt_ctx_t ctx;\n \n \tcmd_is_annotate = !strcmp(argv[0], \"annotate\");\n \n \tgit_config(git_blame_config, NULL);\n+\tinit_revisions(&revs, NULL);\n \tsave_commit_buffer = 0;\n \n-\topt = 0;\n+\tparse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |\n+\t\t\t    PARSE_OPT_KEEP_ARGV0);\n+\tfor (;;) {\n+\t\tint n;\n+\n+\t\tswitch (parse_options_step(&ctx, options, blame_opt_usage)) {\n+\t\tcase PARSE_OPT_HELP:\n+\t\t\texit(129);\n+\t\tcase PARSE_OPT_DONE:\n+\t\t\tgoto parse_done;\n+\t\t}\n+\n+\t\tif (!strcmp(ctx.argv[0], \"--reverse\")) {\n+\t\t\tctx.argv[0] = \"--children\";\n+\t\t\treverse = 1;\n+\t\t}\n+\t\tn = handle_revision_opt(&revs, ctx.argc, ctx.argv,\n+\t\t\t\t\t&ctx.cpidx, ctx.out);\n+\t\tif (n <= 0) {\n+\t\t\terror(\"unknown option `%s'\", ctx.argv[0]);\n+\t\t\tusage_with_options(blame_opt_usage, options);\n+\t\t}\n+\t\tctx.argv += n;\n+\t\tctx.argc -= n;\n+\t}\n+parse_done:\n+\targc = parse_options_end(&ctx);\n+\n \tseen_dashdash = 0;\n \tfor (unk = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (*arg != '-')\n \t\t\tbreak;\n-\t\telse if (!strcmp(\"-b\", arg))\n-\t\t\tblank_boundary = 1;\n-\t\telse if (!strcmp(\"--root\", arg))\n-\t\t\tshow_root = 1;\n-\t\telse if (!strcmp(\"--reverse\", arg)) {\n-\t\t\targv[unk++] = \"--children\";\n-\t\t\treverse = 1;\n-\t\t}\n-\t\telse if (!strcmp(arg, \"--show-stats\"))\n-\t\t\tshow_stats = 1;\n-\t\telse if (!strcmp(\"-c\", arg))\n-\t\t\toutput_option |= OUTPUT_ANNOTATE_COMPAT;\n-\t\telse if (!strcmp(\"-t\", arg))\n-\t\t\toutput_option |= OUTPUT_RAW_TIMESTAMP;\n-\t\telse if (!strcmp(\"-l\", arg))\n-\t\t\toutput_option |= OUTPUT_LONG_OBJECT_NAME;\n-\t\telse if (!strcmp(\"-s\", arg))\n-\t\t\toutput_option |= OUTPUT_NO_AUTHOR;\n-\t\telse if (!strcmp(\"-w\", arg))\n-\t\t\txdl_opts |= XDF_IGNORE_WHITESPACE;\n-\t\telse if (!strcmp(\"-S\", arg) && ++i < argc)\n-\t\t\trevs_file = argv[i];\n-\t\telse if (!prefixcmp(arg, \"-M\")) {\n-\t\t\topt |= PICKAXE_BLAME_MOVE;\n-\t\t\tblame_move_score = parse_score(arg+2);\n-\t\t}\n-\t\telse if (!prefixcmp(arg, \"-C\")) {\n-\t\t\t/*\n-\t\t\t * -C enables copy from removed files;\n-\t\t\t * -C -C enables copy from existing files, but only\n-\t\t\t *       when blaming a new file;\n-\t\t\t * -C -C -C enables copy from existing files for\n-\t\t\t *          everybody\n-\t\t\t */\n-\t\t\tif (opt & PICKAXE_BLAME_COPY_HARDER)\n-\t\t\t\topt |= PICKAXE_BLAME_COPY_HARDEST;\n-\t\t\tif (opt & PICKAXE_BLAME_COPY)\n-\t\t\t\topt |= PICKAXE_BLAME_COPY_HARDER;\n-\t\t\topt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;\n-\t\t\tblame_copy_score = parse_score(arg+2);\n-\t\t}\n-\t\telse if (!prefixcmp(arg, \"-L\")) {\n-\t\t\tif (!arg[2]) {\n-\t\t\t\tif (++i >= argc)\n-\t\t\t\t\tusage(blame_usage);\n-\t\t\t\targ = argv[i];\n-\t\t\t}\n-\t\t\telse\n-\t\t\t\targ += 2;\n-\t\t\tif (bottomtop)\n-\t\t\t\tdie(\"More than one '-L n,m' option given\");\n-\t\t\tbottomtop = arg;\n-\t\t}\n-\t\telse if (!strcmp(\"--contents\", arg)) {\n-\t\t\tif (++i >= argc)\n-\t\t\t\tusage(blame_usage);\n-\t\t\tcontents_from = argv[i];\n-\t\t}\n-\t\telse if (!strcmp(\"--incremental\", arg))\n-\t\t\tincremental = 1;\n-\t\telse if (!strcmp(\"--score-debug\", arg))\n-\t\t\toutput_option |= OUTPUT_SHOW_SCORE;\n-\t\telse if (!strcmp(\"-f\", arg) ||\n-\t\t\t !strcmp(\"--show-name\", arg))\n-\t\t\toutput_option |= OUTPUT_SHOW_NAME;\n-\t\telse if (!strcmp(\"-n\", arg) ||\n-\t\t\t !strcmp(\"--show-number\", arg))\n-\t\t\toutput_option |= OUTPUT_SHOW_NUMBER;\n-\t\telse if (!strcmp(\"-p\", arg) ||\n-\t\t\t !strcmp(\"--porcelain\", arg))\n-\t\t\toutput_option |= OUTPUT_PORCELAIN;\n \t\telse if (!strcmp(\"--\", arg)) {\n \t\t\tseen_dashdash = 1;\n \t\t\ti++;\n@@ -2364,16 +2381,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tif (seen_dashdash) {\n \t\t/* (1) */\n \t\tif (argc <= i)\n-\t\t\tusage(blame_usage);\n+\t\t\tusage_with_options(blame_opt_usage, options);\n \t\tpath = add_prefix(prefix, argv[i]);\n \t\tif (i + 1 == argc - 1) {\n \t\t\tif (unk != 1)\n-\t\t\t\tusage(blame_usage);\n+\t\t\t\tusage_with_options(blame_opt_usage, options);\n \t\t\targv[unk++] = argv[i + 1];\n \t\t}\n \t\telse if (i + 1 != argc)\n \t\t\t/* garbage at end */\n-\t\t\tusage(blame_usage);\n+\t\t\tusage_with_options(blame_opt_usage, options);\n \t}\n \telse {\n \t\tint j;\n@@ -2383,7 +2400,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tif (seen_dashdash) {\n \t\t\t/* (2) */\n \t\t\tif (seen_dashdash + 1 != argc - 1)\n-\t\t\t\tusage(blame_usage);\n+\t\t\t\tusage_with_options(blame_opt_usage, options);\n \t\t\tpath = add_prefix(prefix, argv[seen_dashdash + 1]);\n \t\t\tfor (j = i; j < seen_dashdash; j++)\n \t\t\t\targv[unk++] = argv[j];\n@@ -2391,7 +2408,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\telse {\n \t\t\t/* (3) */\n \t\t\tif (argc <= i)\n-\t\t\t\tusage(blame_usage);\n+\t\t\t\tusage_with_options(blame_opt_usage, options);\n \t\t\tpath = add_prefix(prefix, argv[i]);\n \t\t\tif (i + 1 == argc - 1) {\n \t\t\t\tfinal_commit_name = argv[i + 1];\n@@ -2405,7 +2422,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t\t\t}\n \t\t\t}\n \t\t\telse if (i != argc - 1)\n-\t\t\t\tusage(blame_usage); /* garbage at end */\n+\t\t\t\tusage_with_options(blame_opt_usage, options);\n \n \t\t\tsetup_work_tree();\n \t\t\tif (!has_path_in_work_tree(path))\n@@ -2424,7 +2441,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \targv[unk++] = \"--\"; /* terminate the rev name */\n \targv[unk] = NULL;\n \n-\tinit_revisions(&revs, NULL);\n \tparse_revisions(unk, argv, &revs);\n \tsetup_revisions(&revs, NULL);\n \tmemset(&sb, 0, sizeof(sb));\n-- \n1.5.6.2.352.g416a6\n"},{"id":"82572","messageId":"1215510964-16664-7-git-send-email-madcoder@debian.org","threadId":"14344","inReplyTo":"1215510964-16664-6-git-send-email-madcoder@debian.org","subject":"[PATCH 6/6] git-blame: migrate to incremental parse-option [2/2]","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T09:56:04Z","receivedAt":"2008-07-08T09:56:04Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Now use handle_revision_args instead of parse_revisions, and simplify the\nhandling of old-style arguments a lot thanks to the filtering.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n builtin-blame.c |  132 +++++++++++++++++--------------------------------------\n 1 files changed, 40 insertions(+), 92 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 6256341..bbbaea5 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -2262,8 +2262,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tstruct scoreboard sb;\n \tstruct origin *o;\n \tstruct blame_entry *ent;\n-\tint i, seen_dashdash, unk;\n-\tlong bottom, top, lno;\n+\tlong dashdash_pos, bottom, top, lno;\n \tconst char *final_commit_name = NULL;\n \tenum object_type type;\n \n@@ -2301,6 +2300,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tgit_config(git_blame_config, NULL);\n \tinit_revisions(&revs, NULL);\n \tsave_commit_buffer = 0;\n+\tdashdash_pos = 0;\n \n \tparse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |\n \t\t\t    PARSE_OPT_KEEP_ARGV0);\n@@ -2311,6 +2311,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tcase PARSE_OPT_HELP:\n \t\t\texit(129);\n \t\tcase PARSE_OPT_DONE:\n+\t\t\tif (ctx.argv[0])\n+\t\t\t\tdashdash_pos = ctx.cpidx;\n \t\t\tgoto parse_done;\n \t\t}\n \n@@ -2330,20 +2332,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n parse_done:\n \targc = parse_options_end(&ctx);\n \n-\tseen_dashdash = 0;\n-\tfor (unk = i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tif (*arg != '-')\n-\t\t\tbreak;\n-\t\telse if (!strcmp(\"--\", arg)) {\n-\t\t\tseen_dashdash = 1;\n-\t\t\ti++;\n-\t\t\tbreak;\n-\t\t}\n-\t\telse\n-\t\t\targv[unk++] = arg;\n-\t}\n-\n \tif (!blame_move_score)\n \t\tblame_move_score = BLAME_DEFAULT_MOVE_SCORE;\n \tif (!blame_copy_score)\n@@ -2356,92 +2344,52 @@ parse_done:\n \t *\n \t * The remaining are:\n \t *\n-\t * (1) if seen_dashdash, its either\n-\t *     \"-options -- <path>\" or\n-\t *     \"-options -- <path> <rev>\".\n-\t *     but the latter is allowed only if there is no\n-\t *     options that we passed to revision machinery.\n+\t * (1) if dashdash_pos != 0, its either\n+\t *     \"blame [revisions] -- <path>\" or\n+\t *     \"blame -- <path> <rev>\"\n \t *\n-\t * (2) otherwise, we may have \"--\" somewhere later and\n-\t *     might be looking at the first one of multiple 'rev'\n-\t *     parameters (e.g. \" master ^next ^maint -- path\").\n-\t *     See if there is a dashdash first, and give the\n-\t *     arguments before that to revision machinery.\n-\t *     After that there must be one 'path'.\n+\t * (2) otherwise, its one of the two:\n+\t *     \"blame [revisions] <path>\"\n+\t *     \"blame <path> <rev>\"\n \t *\n-\t * (3) otherwise, its one of the three:\n-\t *     \"-options <path> <rev>\"\n-\t *     \"-options <rev> <path>\"\n-\t *     \"-options <path>\"\n-\t *     but again the first one is allowed only if\n-\t *     there is no options that we passed to revision\n-\t *     machinery.\n+\t * Note that we must strip out <path> from the arguments: we do not\n+\t * want the path pruning but we may want \"bottom\" processing.\n \t */\n-\n-\tif (seen_dashdash) {\n-\t\t/* (1) */\n-\t\tif (argc <= i)\n-\t\t\tusage_with_options(blame_opt_usage, options);\n-\t\tpath = add_prefix(prefix, argv[i]);\n-\t\tif (i + 1 == argc - 1) {\n-\t\t\tif (unk != 1)\n+        if (dashdash_pos) {\n+\t\tswitch (argc - dashdash_pos - 1) {\n+\t\tcase 2: /* (1b) */\n+\t\t\tif (argc != 4)\n \t\t\t\tusage_with_options(blame_opt_usage, options);\n-\t\t\targv[unk++] = argv[i + 1];\n+\t\t\t/* reorder for the new way: <rev> -- <path> */\n+\t\t\targv[1] = argv[3];\n+\t\t\targv[3] = argv[2];\n+\t\t\targv[2] = \"--\";\n+\t\t\tdashdash_pos = 2;\n+\t\t\t/* FALLTHROUGH */\n+\t\tcase 1: /* (1a) */\n+\t\t\tpath = add_prefix(prefix, argv[--argc]);\n+\t\t\targv[argc] = NULL;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tusage_with_options(blame_opt_usage, options);\n \t\t}\n-\t\telse if (i + 1 != argc)\n-\t\t\t/* garbage at end */\n+\t} else {\n+\t\tif (argc < 2)\n \t\t\tusage_with_options(blame_opt_usage, options);\n-\t}\n-\telse {\n-\t\tint j;\n-\t\tfor (j = i; !seen_dashdash && j < argc; j++)\n-\t\t\tif (!strcmp(argv[j], \"--\"))\n-\t\t\t\tseen_dashdash = j;\n-\t\tif (seen_dashdash) {\n-\t\t\t/* (2) */\n-\t\t\tif (seen_dashdash + 1 != argc - 1)\n-\t\t\t\tusage_with_options(blame_opt_usage, options);\n-\t\t\tpath = add_prefix(prefix, argv[seen_dashdash + 1]);\n-\t\t\tfor (j = i; j < seen_dashdash; j++)\n-\t\t\t\targv[unk++] = argv[j];\n+\t\tpath = add_prefix(prefix, argv[argc - 1]);\n+\t\tif (argc == 3 && !has_path_in_work_tree(path)) { /* (2b) */\n+\t\t\tpath = add_prefix(prefix, argv[1]);\n+\t\t\targv[1] = argv[2];\n \t\t}\n-\t\telse {\n-\t\t\t/* (3) */\n-\t\t\tif (argc <= i)\n-\t\t\t\tusage_with_options(blame_opt_usage, options);\n-\t\t\tpath = add_prefix(prefix, argv[i]);\n-\t\t\tif (i + 1 == argc - 1) {\n-\t\t\t\tfinal_commit_name = argv[i + 1];\n-\n-\t\t\t\t/* if (unk == 1) we could be getting\n-\t\t\t\t * old-style\n-\t\t\t\t */\n-\t\t\t\tif (unk == 1 && !has_path_in_work_tree(path)) {\n-\t\t\t\t\tpath = add_prefix(prefix, argv[i + 1]);\n-\t\t\t\t\tfinal_commit_name = argv[i];\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\telse if (i != argc - 1)\n-\t\t\t\tusage_with_options(blame_opt_usage, options);\n+\t\targv[argc - 1] = \"--\";\n+\t\tdashdash_pos = argc - 1;\n \n-\t\t\tsetup_work_tree();\n-\t\t\tif (!has_path_in_work_tree(path))\n-\t\t\t\tdie(\"cannot stat path %s: %s\",\n-\t\t\t\t    path, strerror(errno));\n-\t\t}\n+\t\tsetup_work_tree();\n+\t\tif (!has_path_in_work_tree(path))\n+\t\t\tdie(\"cannot stat path %s: %s\", path, strerror(errno));\n \t}\n \n-\tif (final_commit_name)\n-\t\targv[unk++] = final_commit_name;\n-\n-\t/*\n-\t * Now we got rev and path.  We do not want the path pruning\n-\t * but we may want \"bottom\" processing.\n-\t */\n-\targv[unk++] = \"--\"; /* terminate the rev name */\n-\targv[unk] = NULL;\n-\n-\tparse_revisions(unk, argv, &revs);\n+\thandle_revision_args(&revs, argc, argv, dashdash_pos);\n \tsetup_revisions(&revs, NULL);\n \tmemset(&sb, 0, sizeof(sb));\n \n-- \n1.5.6.2.352.g416a6\n"},{"id":"82578","messageId":"20080708100234.GA19202@artemis.madism.org","threadId":"14344","inReplyTo":"1215510964-16664-1-git-send-email-madcoder@debian.org","subject":"Re: Migration of builtin-blame to parse-option","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T10:02:34Z","receivedAt":"2008-07-08T10:02:34Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"  While rebasing this branch on the latest next, I had a conflict for\nthe patch 1 that I fixed, and I also fixed spacing issues in patches 4\nand 6. The fixed series is on my public git repo[0], but it's really\nlike this one, so I don't see the point in flooding again with it ;)\n\n  [0] git://git.madism.org/~madcoder/git.git#ph/parseopt\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82583","messageId":"487346A1.4040006@viscovery.net","threadId":"14344","inReplyTo":"1215510964-16664-3-git-send-email-madcoder@debian.org","subject":"Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-07-08T10:51:13Z","receivedAt":"2008-07-08T10:51:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Pierre Habouzit schrieb:\n> +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n> +\t\t\tint *unkc, const char **unkv)\n> +{\n> +\tconst char *arg = argv[0];\n> +\n> +\t/* pseudo revision arguments */\n> +\tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n> +\t    !strcmp(arg, \"--tags\") || !strcmp(arg, \"--remotes\") ||\n> +\t    !strcmp(arg, \"--reflog\") || !strcmp(arg, \"--not\") ||\n> +\t    !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\"))\n> +\t{\n> +\t\tunkv[*unkc++] = arg;\n\n+\t\tunkv[(*unkc)++] = arg;\n\nHm?\n\n-- Hannes\n"},{"id":"82584","messageId":"alpine.DEB.1.00.0807081258010.4319@eeepc-johanness","threadId":"14344","inReplyTo":"1215510964-16664-2-git-send-email-madcoder@debian.org","subject":"Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-08T10:59:43Z","receivedAt":"2008-07-08T10:59:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 8 Jul 2008, Pierre Habouzit wrote:\n\n> setup_revisions has been split in two: parse_revisions that does \n> (almost) only argument parsing, to be more like what parse-options can \n> do, and setup_revisions that does the rest.\n\nI do not see the sense of this change, except\n\n- blowing up your patch unnecessarily,\n- making it more inconvenient for users, and\n- adding an opportunity for bugs to creep in.\n\nIn other words, I would like to see the semantics of setup_revisions() \nuntouched.\n\nCiao,\nDscho\n"},{"id":"82585","messageId":"20080708110022.GE19202@artemis.madism.org","threadId":"14344","inReplyTo":"487346A1.4040006@viscovery.net","subject":"Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T11:00:22Z","receivedAt":"2008-07-08T11:00:22Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:\n> Pierre Habouzit schrieb:\n> > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n> > +\t\t\tint *unkc, const char **unkv)\n> > +{\n> > +\tconst char *arg = argv[0];\n> > +\n> > +\t/* pseudo revision arguments */\n> > +\tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n> > +\t    !strcmp(arg, \"--tags\") || !strcmp(arg, \"--remotes\") ||\n> > +\t    !strcmp(arg, \"--reflog\") || !strcmp(arg, \"--not\") ||\n> > +\t    !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\"))\n> > +\t{\n> > +\t\tunkv[*unkc++] = arg;\n> \n> +\t\tunkv[(*unkc)++] = arg;\n\n  Huh right. Good catch, I wonder why the testsuite failed to see that.\nAnyways I pushed a fix on my public repo.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82586","messageId":"20080708110624.GF19202@artemis.madism.org","threadId":"14344","inReplyTo":"alpine.DEB.1.00.0807081258010.4319@eeepc-johanness","subject":"Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T11:06:24Z","receivedAt":"2008-07-08T11:06:24Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 8 Jul 2008, Pierre Habouzit wrote:\n> \n> > setup_revisions has been split in two: parse_revisions that does \n> > (almost) only argument parsing, to be more like what parse-options can \n> > do, and setup_revisions that does the rest.\n> \n> I do not see the sense of this change, except\n\n  Well, it's required to remove \"parse_revisions\" at once if we one day\nreach the goal of having only parse-opt based parsers.\n\n> - blowing up your patch unnecessarily,\n\n> - making it more inconvenient for users, and\n  It makes it convenient for parse-opt based parser, and it makes it\nconvenient for people that don't have arguments to parse (and those are\nmany).\n\n> - adding an opportunity for bugs to creep in.\n> \n> In other words, I would like to see the semantics of setup_revisions() \n> untouched.\n\n  FWIW like said I already submitted that patch under different versions\nin the past, and I seem to recall that it was welcomed as it splits the\nrev_info final fixups from the argument parsing which are quite\ndifferent steps.\n\n  FWIW I really like the new semantics better. If you're worried about\nthe fact that people may be confused about the new semantics, fine,\nlet's rename it then.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"82588","messageId":"20080708112532.GG19202@artemis.madism.org","threadId":"14344","inReplyTo":"20080708110022.GE19202@artemis.madism.org","subject":"Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T11:25:32Z","receivedAt":"2008-07-08T11:25:32Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Jul 08, 2008 at 11:00:22AM +0000, Pierre Habouzit wrote:\n> On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:\n> > Pierre Habouzit schrieb:\n> > > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n> > > +\t\t\tint *unkc, const char **unkv)\n> > > +{\n> > > +\tconst char *arg = argv[0];\n> > > +\n> > > +\t/* pseudo revision arguments */\n> > > +\tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n> > > +\t    !strcmp(arg, \"--tags\") || !strcmp(arg, \"--remotes\") ||\n> > > +\t    !strcmp(arg, \"--reflog\") || !strcmp(arg, \"--not\") ||\n> > > +\t    !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\"))\n> > > +\t{\n> > > +\t\tunkv[*unkc++] = arg;\n> > \n> > +\t\tunkv[(*unkc)++] = arg;\n> \n>   Huh right. Good catch, I wonder why the testsuite failed to see that.\n\n  Okay, I know why, there is another bug that was hiding this one,\nparse_revisions whas doing argv[left++] = arg itself too, which was\nfixing this mistake. I've pushed it again, and here is the fixed patch\n3/6 for reviewing.\n\n-----------8<-----------\nFrom d0a062617526128c4ec0245614eb9916b7996c38 Mon Sep 17 00:00:00 2001\nFrom: Pierre Habouzit <madcoder@debian.org>\nDate: Tue, 8 Jul 2008 11:00:02 +0200\nSubject: [PATCH] revisions: split handle_revision_args from parse_revisions.\n\nThis new function is meant to only parse non option revision arguments in\na row. IOW it's meant to parse what remains from a parse-opt filtering of\nthe argument list, with the knowledge of the \"--\" position (0 means none).\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n revision.c |   97 +++++++++++++++++++++++++++++++++++------------------------\n revision.h |    1 +\n 2 files changed, 58 insertions(+), 40 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 99b3cc9..6958725 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1241,39 +1241,15 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n \treturn 1;\n }\n \n-/*\n- * Parse revision information, filling in the \"rev_info\" structure,\n- * and removing the used arguments from the argument list.\n- *\n- * Returns the number of arguments left that weren't recognized\n- * (which are also moved to the head of the argument list)\n- */\n-int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n+int handle_revision_args(struct rev_info *revs,\n+\t\t\t int argc, const char **argv, int dashdash_pos)\n {\n-\tint i, flags, seen_dashdash;\n-\tconst char **unrecognized = argv + 1;\n-\tint left = 1;\n+\tint i, left, flags = 0;\n \n-\t/* First, search for \"--\" */\n-\tseen_dashdash = 0;\n-\tfor (i = 1; i < argc; i++) {\n+\tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-\t\tif (strcmp(arg, \"--\"))\n-\t\t\tcontinue;\n-\t\targv[i] = NULL;\n-\t\targc = i;\n-\t\tif (argv[i + 1])\n-\t\t\trevs->prune_data = get_pathspec(revs->prefix, argv + i + 1);\n-\t\tseen_dashdash = 1;\n-\t\tbreak;\n-\t}\n \n-\tflags = 0;\n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n \t\tif (*arg == '-') {\n-\t\t\tint opts;\n-\n \t\t\tif (!strcmp(arg, \"--all\")) {\n \t\t\t\thandle_refs(revs, flags, for_each_ref);\n \t\t\t\tcontinue;\n@@ -1306,23 +1282,22 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\t\trevs->no_walk = 0;\n \t\t\t\tcontinue;\n \t\t\t}\n-\n-\t\t\topts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);\n-\t\t\tif (opts > 0) {\n-\t\t\t\ti += opts - 1;\n-\t\t\t\tcontinue;\n+\t\t\tif (i == dashdash_pos) {\n+\t\t\t\targv[i] = NULL;\n+\t\t\t\targc = i;\n+\t\t\t\tif (argv[i + 1])\n+\t\t\t\t\trevs->prune_data = get_pathspec(revs->prefix, argv + i + 1);\n+\t\t\t\tbreak;\n \t\t\t}\n-\t\t\tif (opts < 0)\n-\t\t\t\texit(128);\n-\t\t\t*unrecognized++ = arg;\n-\t\t\tleft++;\n+\n+\t\t\targv[left++] = arg;\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (handle_revision_arg(arg, revs, flags, seen_dashdash)) {\n+\t\tif (handle_revision_arg(arg, revs, flags, dashdash_pos)) {\n \t\t\tint j;\n-\t\t\tif (seen_dashdash || *arg == '^')\n-\t\t\t\tdie(\"bad revision '%s'\", arg);\n+\t\t\tif (dashdash_pos || *arg == '^')\n+\t\t\t\treturn error(\"bad revision '%s'\", arg);\n \n \t\t\t/* If we didn't have a \"--\":\n \t\t\t * (1) all filenames must exist;\n@@ -1338,9 +1313,51 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n \t\t\tbreak;\n \t\t}\n \t}\n+\n \treturn left;\n }\n \n+/*\n+ * Parse revision information, filling in the \"rev_info\" structure,\n+ * and removing the used arguments from the argument list.\n+ *\n+ * Returns the number of arguments left that weren't recognized\n+ * (which are also moved to the head of the argument list)\n+ */\n+int parse_revisions(int argc, const char **argv, struct rev_info *revs)\n+{\n+\tint i, left, dashdash_pos = 0;\n+\n+\t/* First, filter out revision options and look for \"--\" */\n+\tfor (left = i = 1; i < argc; i++) {\n+\t\tconst char *arg = argv[i];\n+\t\tint opts;\n+\n+\t\tif (arg[0] != '-') {\n+\t\t\targv[left++] = arg;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!strcmp(arg, \"--\")) {\n+\t\t\tdashdash_pos = left;\n+\t\t\tmemcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));\n+\t\t\tleft += argc - i;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n+\t\tif (opts > 0) {\n+\t\t\ti += opts - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (opts < 0)\n+\t\t\texit(128);\n+\t}\n+\targv[left] = NULL;\n+\n+\treturn handle_revision_args(revs, left, argv, dashdash_pos);\n+}\n+\n void setup_revisions(struct rev_info *revs, const char *def)\n {\n \tif (revs->def == NULL)\ndiff --git a/revision.h b/revision.h\nindex 46ab713..c0f5df0 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -125,6 +125,7 @@ extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);\n extern void setup_revisions(struct rev_info *revs, const char *def);\n extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,\n \t\t\t       int *unkc, const char **unkv);\n+extern int handle_revision_args(struct rev_info *revs, int argc, const char **argv, int dashdash_pos);\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);\n \n extern int prepare_revision_walk(struct rev_info *revs);\n-- \n1.5.6.2.399.g15e15\n\n"},{"id":"82591","messageId":"alpine.DEB.1.00.0807081339270.4319@eeepc-johanness","threadId":"14344","inReplyTo":"20080708110624.GF19202@artemis.madism.org","subject":"Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-08T11:43:15Z","receivedAt":"2008-07-08T11:43:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 8 Jul 2008, Pierre Habouzit wrote:\n\n> On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:\n> \n> > On Tue, 8 Jul 2008, Pierre Habouzit wrote:\n> > \n> > > setup_revisions has been split in two: parse_revisions that does \n> > > (almost) only argument parsing, to be more like what parse-options \n> > > can do, and setup_revisions that does the rest.\n> > \n> > I do not see the sense of this change, except\n> \n> Well, it's required to remove \"parse_revisions\" at once if we one day \n> reach the goal of having only parse-opt based parsers.\n\nWe can do that, then.\n\nWe do not need to do that, now.\n\nIn the meantime, your patch series of already pretty large patches gets \neven larger, which prevents me from being able to review them.\n\nSad,\nDscho\n\nP.S.: Please do not pull things at me like \"it is so convenient to be able \nto drop \"0, NULL, \" for many sites\".  That is not even funny.  This case \ndoes not need more convenience.  The other cases do.\n"},{"id":"82596","messageId":"20080708124832.GH19202@artemis.madism.org","threadId":"14344","inReplyTo":"alpine.DEB.1.00.0807081339270.4319@eeepc-johanness","subject":"Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-08T12:48:32Z","receivedAt":"2008-07-08T12:48:32Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On mar, jui 08, 2008 at 11:43:15 +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 8 Jul 2008, Pierre Habouzit wrote:\n> \n> > On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:\n> > \n> > > On Tue, 8 Jul 2008, Pierre Habouzit wrote:\n> > > \n> > > > setup_revisions has been split in two: parse_revisions that does \n> > > > (almost) only argument parsing, to be more like what parse-options \n> > > > can do, and setup_revisions that does the rest.\n> > > \n> > > I do not see the sense of this change, except\n> > \n> > Well, it's required to remove \"parse_revisions\" at once if we one day \n> > reach the goal of having only parse-opt based parsers.\n> \n> We can do that, then.\n> \n> We do not need to do that, now.\n\n  hmmm right, I may be able to cook a simpler series without that\nindeed. Ill repost a new one with this idea.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"}]}