{"thread":{"id":"12507","subject":"[PATCH] revisions: refactor init_revisions and setup_revisions.","startedAt":"2008-03-04T23:19:45Z","lastAt":"2008-03-10T20:44:07Z","messageCount":5,"participants":["Pierre Habouzit","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71011","messageId":"20080304231945.GC24131@artemis.madism.org","threadId":"12507","inReplyTo":null,"subject":"[PATCH] revisions: refactor init_revisions and setup_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-04T23:19:45Z","receivedAt":"2008-03-04T23:19:45Z","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\nThe refactor consists into the following steps:\n* init_revisions now takes the --default argument to initialize .def\n  properly, instead of getting it through setup_revisions.\n* setup_revisions has been split in two: parse_revisions that does (almost)\n  only argument parsing, to be more like what parse-options can do, and\n  setup_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\n  This is a required patch for revisions parse-optification that splits\n  the logic of revision arguments parsing in parsing, and post-processing.\n\n  The aim is to replace parsing with parse-opt at some point.\n\n  The final version (using parse-options) will probably need more rework\n  in revisions.[hc], but I'd like to avoid rebasing this patch over and\n  over, and I'd be glad if it's merged now, as it's not _that_\n  intrusive.\n\n  I've been using a git with this patch for a week without issues.\n\n\n builtin-add.c           |    4 ++--\n builtin-blame.c         |    5 +++--\n builtin-checkout.c      |    7 ++++---\n builtin-commit.c        |    8 ++++----\n builtin-diff-files.c    |    8 +++++---\n builtin-diff-index.c    |    5 +++--\n builtin-diff-tree.c     |    5 +++--\n builtin-diff.c          |    8 +++++---\n builtin-fast-export.c   |    5 +++--\n builtin-fmt-merge-msg.c |    4 ++--\n builtin-log.c           |   20 +++++++++++---------\n builtin-pack-objects.c  |    5 +++--\n builtin-prune.c         |    2 +-\n builtin-reflog.c        |    2 +-\n builtin-rev-list.c      |    5 +++--\n builtin-revert.c        |    4 ++--\n builtin-shortlog.c      |    5 +++--\n bundle.c                |   10 ++++++----\n diff-lib.c              |    2 +-\n http-push.c             |    5 +++--\n revision.c              |   36 +++++++++++++++++++-----------------\n revision.h              |    7 +++++--\n upload-pack.c           |    6 +++---\n wt-status.c             |   12 ++++++------\n 24 files changed, 101 insertions(+), 79 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 820110e..3757369 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -108,8 +108,8 @@ static void update_callback(struct diff_queue_struct *q,\n void add_files_to_cache(int verbose, const char *prefix, const char **pathspec)\n {\n \tstruct rev_info rev;\n-\tinit_revisions(&rev, prefix);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n+\tinit_revisions(&rev, prefix, NULL);\n+\tsetup_revisions(&rev);\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 bfd562d..ac5c5eb 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -2323,8 +2323,9 @@ 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-\tsetup_revisions(unk, argv, &revs, NULL);\n+\tinit_revisions(&revs, NULL, NULL);\n+\tparse_revisions(unk, argv, &revs);\n+\tsetup_revisions(&revs);\n \tmemset(&sb, 0, sizeof(sb));\n \n \t/*\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 6b08016..ff92ef6 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -130,7 +130,7 @@ static void show_local_changes(struct object *head)\n {\n \tstruct rev_info rev;\n \t/* I think we want full paths, even if we're in a subdirectory. */\n-\tinit_revisions(&rev, NULL);\n+\tinit_revisions(&rev, NULL, NULL);\n \trev.abbrev = 0;\n \trev.diffopt.output_format |= DIFF_FORMAT_NAME_STATUS;\n \tadd_pending_object(&rev, head, NULL);\n@@ -345,8 +345,9 @@ static void report_tracking(struct branch_info *new, struct checkout_opts *opts)\n \tstrcpy(symmetric + 40, \"...\");\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+\tinit_revisions(&revs, NULL, NULL);\n+\tparse_revisions(rev_argc, rev_argv, &revs);\n+\tsetup_revisions(&revs);\n \tprepare_revision_walk(&revs);\n \n \t/* ... and count the commits on each side. */\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex f49c22e..48dbcaa 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -522,9 +522,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\tif (get_sha1(parent, sha1))\n \t\t\tcommitable = !!active_nr;\n \t\telse {\n-\t\t\tinit_revisions(&rev, \"\");\n+\t\t\tinit_revisions(&rev, \"\", parent);\n \t\t\trev.abbrev = 0;\n-\t\t\tsetup_revisions(0, NULL, &rev, parent);\n+\t\t\tsetup_revisions(&rev);\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@@ -798,8 +798,8 @@ static void print_summary(const char *prefix, const unsigned char *sha1)\n \tif (!commit || parse_commit(commit))\n \t\tdie(\"could not parse newly created commit\");\n \n-\tinit_revisions(&rev, prefix);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n+\tinit_revisions(&rev, prefix, NULL);\n+\tsetup_revisions(&rev);\n \n \trev.abbrev = 0;\n \trev.diff = 1;\ndiff --git a/builtin-diff-files.c b/builtin-diff-files.c\nindex 4abe3c2..1f6f123 100644\n--- a/builtin-diff-files.c\n+++ b/builtin-diff-files.c\n@@ -20,14 +20,16 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \tint result;\n \n \tprefix = setup_git_directory_gently(&nongit);\n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, NULL);\n \tgit_config(git_diff_basic_config); /* no \"diff\" UI options */\n \trev.abbrev = 0;\n \n \tif (!setup_diff_no_index(&rev, argc, argv, nongit, prefix))\n \t\targc = 0;\n-\telse\n-\t\targc = setup_revisions(argc, argv, &rev, NULL);\n+\telse {\n+\t\targc = parse_revisions(argc, argv, &rev);\n+\t\tsetup_revisions(&rev);\n+\t}\n \tif (!rev.diffopt.output_format)\n \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n \tresult = run_diff_files_cmd(&rev, argc, argv);\ndiff --git a/builtin-diff-index.c b/builtin-diff-index.c\nindex 2b955de..43dc3bc 100644\n--- a/builtin-diff-index.c\n+++ b/builtin-diff-index.c\n@@ -16,11 +16,12 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n \tint i;\n \tint result;\n \n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, NULL);\n \tgit_config(git_diff_basic_config); /* 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);\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 832797f..3b03d7a 100644\n--- a/builtin-diff-tree.c\n+++ b/builtin-diff-tree.c\n@@ -67,12 +67,13 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \tstatic struct rev_info *opt = &log_tree_opt;\n \tint read_stdin = 0;\n \n-\tinit_revisions(opt, prefix);\n+\tinit_revisions(opt, prefix, NULL);\n \tgit_config(git_diff_basic_config); /* no \"diff\" UI options */\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);\n \n \twhile (--argc > 0) {\n \t\tconst char *arg = *++argv;\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 444ff2f..19f4111 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -239,13 +239,15 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tif (diff_use_color_default == -1)\n \t\tdiff_use_color_default = git_use_color_default;\n \n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, NULL);\n \trev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;\n \n \tif (!setup_diff_no_index(&rev, argc, argv, nongit, prefix))\n \t\targc = 0;\n-\telse\n-\t\targc = setup_revisions(argc, argv, &rev, NULL);\n+\telse {\n+\t\targc = parse_revisions(argc, argv, &rev);\n+\t\tsetup_revisions(&rev);\n+\t}\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 e1c5630..4b788e5 100755\n--- a/builtin-fast-export.c\n+++ b/builtin-fast-export.c\n@@ -374,8 +374,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t/* we handle encodings */\n \tgit_config(git_default_config);\n \n-\tinit_revisions(&revs, prefix);\n-\targc = setup_revisions(argc, argv, &revs, NULL);\n+\tinit_revisions(&revs, prefix, NULL);\n+\targc = parse_revisions(argc, argv, &revs);\n+\tsetup_revisions(&revs);\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 ebb3f37..9b84482 100644\n--- a/builtin-fmt-merge-msg.c\n+++ b/builtin-fmt-merge-msg.c\n@@ -182,7 +182,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);\n \trev->ignore_merges = 1;\n \tadd_pending_object(rev, branch, name);\n \tadd_pending_object(rev, &head->object, \"^HEAD\");\n@@ -339,7 +339,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)\n \t\tstruct rev_info rev;\n \n \t\thead = lookup_commit(head_sha1);\n-\t\tinit_revisions(&rev, prefix);\n+\t\tinit_revisions(&rev, prefix, NULL);\n \t\trev.commit_format = CMIT_FMT_ONELINE;\n \t\trev.ignore_merges = 1;\n \t\trev.limited = 1;\ndiff --git a/builtin-log.c b/builtin-log.c\nindex fe8fc6f..30d13d3 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -58,7 +58,8 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n-\targc = setup_revisions(argc, argv, rev, \"HEAD\");\n+\targc = parse_revisions(argc, argv, rev);\n+\tsetup_revisions(rev);\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@@ -243,7 +244,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n \tif (diff_use_color_default == -1)\n \t\tdiff_use_color_default = git_use_color_default;\n \n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, \"HEAD\");\n \trev.diff = 1;\n \trev.simplify_history = 0;\n \tcmd_log_init(argc, argv, prefix, &rev);\n@@ -319,7 +320,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \tif (diff_use_color_default == -1)\n \t\tdiff_use_color_default = git_use_color_default;\n \n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, \"HEAD\");\n \trev.diff = 1;\n \trev.combine_merges = 1;\n \trev.dense_combined_merges = 1;\n@@ -383,7 +384,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \tif (diff_use_color_default == -1)\n \t\tdiff_use_color_default = git_use_color_default;\n \n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, \"HEAD\");\n \tinit_reflog_walk(&rev.reflog_info);\n \trev.abbrev_commit = 1;\n \trev.verbose_header = 1;\n@@ -415,7 +416,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \tif (diff_use_color_default == -1)\n \t\tdiff_use_color_default = git_use_color_default;\n \n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, \"HEAD\");\n \trev.always_show_header = 1;\n \tcmd_log_init(argc, argv, prefix, &rev);\n \treturn cmd_log_walk(&rev);\n@@ -601,7 +602,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n \tinit_patch_ids(ids);\n \n \t/* given a range a..b get all patch ids for b..a */\n-\tinit_revisions(&check_rev, prefix);\n+\tinit_revisions(&check_rev, prefix, NULL);\n \to1->flags ^= UNINTERESTING;\n \to2->flags ^= UNINTERESTING;\n \tadd_pending_object(&check_rev, o1, \"o1\");\n@@ -756,7 +757,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buf;\n \n \tgit_config(git_format_config);\n-\tinit_revisions(&rev, prefix);\n+\tinit_revisions(&rev, prefix, \"HEAD\");\n \trev.commit_format = CMIT_FMT_EMAIL;\n \trev.verbose_header = 1;\n \trev.diff = 1;\n@@ -900,7 +901,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);\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \n@@ -1095,7 +1097,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \t\tusage(cherry_usage);\n \t}\n \n-\tinit_revisions(&revs, prefix);\n+\tinit_revisions(&revs, prefix, NULL);\n \trevs.diff = 1;\n \trevs.combine_merges = 0;\n \trevs.ignore_merges = 1;\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 2799e68..94d0adc 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1871,9 +1871,10 @@ static void get_object_list(int ac, const char **av)\n \tchar line[1000];\n \tint flags = 0;\n \n-\tinit_revisions(&revs, NULL);\n+\tinit_revisions(&revs, NULL, NULL);\n \tsave_commit_buffer = 0;\n-\tsetup_revisions(ac, av, &revs, NULL);\n+\tparse_revisions(ac, av, &revs);\n+\tsetup_revisions(&revs);\n \n \twhile (fgets(line, sizeof(line), stdin) != NULL) {\n \t\tint len = strlen(line);\ndiff --git a/builtin-prune.c b/builtin-prune.c\nindex bb8ead9..c18f8c2 100644\n--- a/builtin-prune.c\n+++ b/builtin-prune.c\n@@ -146,7 +146,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)\n \t}\n \n \tsave_commit_buffer = 0;\n-\tinit_revisions(&revs, prefix);\n+\tinit_revisions(&revs, prefix, NULL);\n \tmark_reachable_objects(&revs, 1);\n \n \tprune_object_dir(get_object_directory());\ndiff --git a/builtin-reflog.c b/builtin-reflog.c\nindex 280e24e..133f6f3 100644\n--- a/builtin-reflog.c\n+++ b/builtin-reflog.c\n@@ -399,7 +399,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\tbreak;\n \t}\n \tif (cb.stalefix) {\n-\t\tinit_revisions(&cb.revs, prefix);\n+\t\tinit_revisions(&cb.revs, prefix, NULL);\n \t\tif (cb.verbose)\n \t\t\tprintf(\"Marking reachable objects...\");\n \t\tmark_reachable_objects(&cb.revs, 0);\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex d0a1416..2a02b07 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -546,10 +546,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \n \tgit_config(git_default_config);\n-\tinit_revisions(&revs, prefix);\n+\tinit_revisions(&revs, prefix, NULL);\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);\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 607a2f0..e538dc1 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -251,8 +251,8 @@ static char *help_msg(const unsigned char *sha1)\n 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+\tinit_revisions(&rev, NULL, \"HEAD\");\n+\tsetup_revisions(&rev);\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 af31aba..2966bdd 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -252,8 +252,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \t\targv++;\n \t\targc--;\n \t}\n-\tinit_revisions(&rev, prefix);\n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\tinit_revisions(&rev, prefix, NULL);\n+\targc = parse_revisions(argc, argv, &rev);\n+\tsetup_revisions(&rev);\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \ndiff --git a/bundle.c b/bundle.c\nindex 0ba5df1..24d5009 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -104,7 +104,7 @@ int verify_bundle(struct bundle_header *header, int verbose)\n \tint i, ret = 0, req_nr;\n \tconst char *message = \"Repository lacks these prerequisite commits:\";\n \n-\tinit_revisions(&revs, NULL);\n+\tinit_revisions(&revs, NULL, NULL);\n \tfor (i = 0; i < p->nr; i++) {\n \t\tstruct ref_list_entry *e = p->list + i;\n \t\tstruct object *o = parse_object(e->sha1);\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);\n \n \tmemset(&refs, 0, sizeof(struct object_array));\n \tfor (i = 0; i < revs.pending.nr; i++) {\n@@ -192,7 +193,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \n \t/* init revs to list objects for pack-objects later */\n \tsave_commit_buffer = 0;\n-\tinit_revisions(&revs, NULL);\n+\tinit_revisions(&revs, NULL, NULL);\n \n \t/* write prerequisites */\n \tmemcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));\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);\n \tif (argc > 1)\n \t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n \ndiff --git a/diff-lib.c b/diff-lib.c\nindex 4581b59..76e1ce2 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -775,7 +775,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \t}\n \tactive_nr = dst - active_cache;\n \n-\tinit_revisions(&revs, NULL);\n+\tinit_revisions(&revs, NULL, NULL);\n \trevs.prune_data = opt->paths;\n \ttree = parse_tree_indirect(tree_sha1);\n \tif (!tree)\ndiff --git a/http-push.c b/http-push.c\nindex 5b23038..65d9f5b 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -2382,8 +2382,9 @@ int main(int argc, char **argv)\n \t\t\tcommit_argv[3] = old_sha1_hex;\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\tinit_revisions(&revs, setup_git_directory(), NULL);\n+\t\tparse_revisions(commit_argc, commit_argv, &revs);\n+\t\tsetup_revisions(&revs);\n \t\tfree(new_sha1_hex);\n \t\tif (old_sha1_hex) {\n \t\t\tfree(old_sha1_hex);\ndiff --git a/revision.c b/revision.c\nindex 63bf2c5..bab6228 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -717,7 +717,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)\n \treturn 1;\n }\n \n-void init_revisions(struct rev_info *revs, const char *prefix)\n+void init_revisions(struct rev_info *revs, const char *prefix, const char *def)\n {\n \tmemset(revs, 0, sizeof(*revs));\n \n@@ -731,6 +731,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)\n \trevs->lifo = 1;\n \trevs->dense = 1;\n \trevs->prefix = prefix;\n+\trevs->def = def;\n \trevs->max_age = -1;\n \trevs->min_age = -1;\n \trevs->skip_count = -1;\n@@ -892,8 +893,7 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token\n \t\topt->regflags = REG_NEWLINE;\n \t\trevs->grep_filter = opt;\n \t}\n-\tappend_grep_pattern(revs->grep_filter, ptn,\n-\t\t\t    \"command line\", 0, what);\n+\tappend_grep_pattern(revs->grep_filter, ptn, \"command line\", 0, what);\n }\n \n static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)\n@@ -936,9 +936,9 @@ 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@@ -959,7 +959,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@@ -1047,11 +1047,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@@ -1315,18 +1315,23 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \tif (revs->grep_filter) {\n \t\trevs->grep_filter->regflags |= regflags;\n \t\trevs->grep_filter->fixed = fixed;\n+\t\trevs->grep_filter->all_match = all_match;\n \t}\n+\treturn left;\n+}\n \n-\tif (show_merge)\n+void setup_revisions(struct rev_info *revs)\n+{\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@@ -1360,14 +1365,11 @@ 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 \tif (revs->reverse && revs->reflog_info)\n \t\tdie(\"cannot combine --reverse with --walk-reflogs\");\n-\n-\treturn left;\n }\n \n int prepare_revision_walk(struct rev_info *revs)\ndiff --git a/revision.h b/revision.h\nindex c8b3b94..9f727d7 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -25,6 +25,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@@ -64,6 +65,7 @@ struct rev_info {\n \n \t/* Format info */\n \tunsigned int\tshown_one:1,\n+\t\t\tshow_merge:1,\n \t\t\tabbrev_commit:1;\n \tenum date_mode date_mode;\n \n@@ -108,8 +110,9 @@ struct rev_info {\n 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 void init_revisions(struct rev_info *revs, const char *prefix, 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);\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 660134a..8adf0f8 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -106,7 +106,7 @@ static int do_rev_list(int fd, void *create_full_pack)\n \tpack_pipe = fdopen(fd, \"w\");\n \tif (create_full_pack)\n \t\tuse_thin_pack = 0; /* no point doing it */\n-\tinit_revisions(&revs, NULL);\n+\tinit_revisions(&revs, NULL, NULL);\n \trevs.tag_objects = 1;\n \trevs.tree_objects = 1;\n \trevs.blob_objects = 1;\n@@ -115,7 +115,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@@ -128,8 +128,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);\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 32d780a..bf3df7c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -241,8 +241,8 @@ static void wt_status_print_initial(struct wt_status *s)\n 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+\tinit_revisions(&rev, NULL, s->reference);\n+\tsetup_revisions(&rev);\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@@ -255,8 +255,8 @@ static void wt_status_print_updated(struct wt_status *s)\n 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+\tinit_revisions(&rev, \"\", NULL);\n+\tsetup_revisions(&rev);\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@@ -322,8 +322,8 @@ static void wt_status_print_verbose(struct wt_status *s)\n \tif (saved_stdout < 0 ||dup2(fileno(s->fp), STDOUT_FILENO) < 0)\n \t\tdie(\"couldn't redirect stdout\\n\");\n \n-\tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, s->reference);\n+\tinit_revisions(&rev, NULL, s->reference);\n+\tsetup_revisions(&rev);\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \trev.diffopt.detect_rename = 1;\n \trun_diff_index(&rev, 1);\n-- \n1.5.4.3.534.g7c43.dirty\n\n"},{"id":"71558","messageId":"7v63vvkplm.fsf@gitster.siamese.dyndns.org","threadId":"12507","inReplyTo":"20080304231945.GC24131@artemis.madism.org","subject":"Re: [PATCH] revisions: refactor init_revisions and setup_revisions.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-10T07:25:25Z","receivedAt":"2008-03-10T07:25:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n    struct rev_info gains two new fields:\n    \n     * .def to store --default argument;\n     * .show_merge 1-bit field.\n    \n    The refactor consists of the following steps:\n    \n     * init_revisions now takes the --default argument to initialize .def\n       properly, instead of getting it through setup_revisions.\n    \n     * setup_revisions has been split in two:\n    \n       - parse_revisions that does (almost) only argument parsing, to be more\n         like what parse-options can do, and\n    \n       - setup_revisions that does the rest.\n    \n    Many places had no arguments to pass to setup_revisions, and those don't\n    use parse_revisions at all.\n\nConceptually, I like the split up into three, which are:\n\n - initializes the structure (init_revisions);\n\n - parses the user arguments (parse_revisions -- the naming could probably\n   be improved, though);\n\n - prepares the derived/derivable fields for the real work\n   (setup_revisions).\n\nWas there a particular reason you moved default to init_revisions() and\nnot kept it with setup_revisions()?  All the callers of cmd_log_init()\nneeds to say \"I want to default to HEAD\" because of it, instead of letting\ncmd_log_init() say it at only one place.  Also the caller needs to decide\nupfront before calling init_revisions() what the default should be.\n\n> Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n> ---\n>\n>   This is a required patch for revisions parse-optification that splits\n>   the logic of revision arguments parsing in parsing, and post-processing.\n>\n>   The aim is to replace parsing with parse-opt at some point.\n>\n>   The final version (using parse-options) will probably need more rework\n>   in revisions.[hc], but I'd like to avoid rebasing this patch over and\n>   over, and I'd be glad if it's merged now, as it's not _that_\n>   intrusive.\n\n>   I've been using a git with this patch for a week without issues.\n\n... which is a nice assuring comment ;-)\n\n> @@ -892,8 +893,7 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token\n>  \t\topt->regflags = REG_NEWLINE;\n>  \t\trevs->grep_filter = opt;\n>  \t}\n> -\tappend_grep_pattern(revs->grep_filter, ptn,\n> -\t\t\t    \"command line\", 0, what);\n> +\tappend_grep_pattern(revs->grep_filter, ptn, \"command line\", 0, what);\n>  }\n\nMade me go \"huh?\".\n\nBut everything else looked pretty mechanical conversion, and in that sense\nit is not that intrusive.\n\nApplying this to 'master' and then merging 'pu' shows that there are a few\ntopics that are cooking that would conflict with this change.  Merging\n'next' seems to go cleanly (I haven't checked the result), so it is not\ntoo bad for me to carrry this at this moment, if we were not this close to\nthe rc freeze.  I dunno.\n"},{"id":"71562","messageId":"20080310084920.GA2798@artemis.madism.org","threadId":"12507","inReplyTo":"7v63vvkplm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] revisions: refactor init_revisions and setup_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-10T08:49:21Z","receivedAt":"2008-03-10T08:49:21Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Mar 10, 2008 at 07:25:25AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n>     struct rev_info gains two new fields:\n>     \n>      * .def to store --default argument;\n>      * .show_merge 1-bit field.\n>     \n>     The refactor consists of the following steps:\n>     \n>      * init_revisions now takes the --default argument to initialize .def\n>        properly, instead of getting it through setup_revisions.\n>     \n>      * setup_revisions has been split in two:\n>     \n>        - parse_revisions that does (almost) only argument parsing, to be more\n>          like what parse-options can do, and\n>     \n>        - setup_revisions that does the rest.\n>     \n>     Many places had no arguments to pass to setup_revisions, and those don't\n>     use parse_revisions at all.\n> \n> Conceptually, I like the split up into three, which are:\n> \n>  - initializes the structure (init_revisions);\n> \n>  - parses the user arguments (parse_revisions -- the naming could probably\n>    be improved, though);\n\n  Well, the goal is to replace that bit with a parse_options\nstructure/macro at some point, so the naming isn't really important.\n\n>  - prepares the derived/derivable fields for the real work\n>    (setup_revisions).\n> \n> Was there a particular reason you moved default to init_revisions() and\n> not kept it with setup_revisions()?  All the callers of cmd_log_init()\n> needs to say \"I want to default to HEAD\" because of it, instead of letting\n> cmd_log_init() say it at only one place.  Also the caller needs to decide\n> upfront before calling init_revisions() what the default should be.\n\n  Yes, because the revision parsing has side effects on that default\nparameter (the --default command line switch).\n\n  We could of course have a .def member in the struct rev_info, and use\nthe one passed to setup_revisions then if it's still NULL, but it\ndoesn't really makes sense to me, and I don't really see a problem with\nsaying at init time that you'll default to \"HEAD\". Though if you really\ndislike it that much, I squash a patch that does that on top of it.\n\n> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n> > ---\n> >\n> >   This is a required patch for revisions parse-optification that splits\n> >   the logic of revision arguments parsing in parsing, and post-processing.\n> >\n> >   The aim is to replace parsing with parse-opt at some point.\n> >\n> >   The final version (using parse-options) will probably need more rework\n> >   in revisions.[hc], but I'd like to avoid rebasing this patch over and\n> >   over, and I'd be glad if it's merged now, as it's not _that_\n> >   intrusive.\n> \n> >   I've been using a git with this patch for a week without issues.\n> \n> .... which is a nice assuring comment ;-)\n\n  Well it has been even more since :)\n\n> > @@ -892,8 +893,7 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token\n> >  \t\topt->regflags = REG_NEWLINE;\n> >  \t\trevs->grep_filter = opt;\n> >  \t}\n> > -\tappend_grep_pattern(revs->grep_filter, ptn,\n> > -\t\t\t    \"command line\", 0, what);\n> > +\tappend_grep_pattern(revs->grep_filter, ptn, \"command line\", 0, what);\n> >  }\n> \n> Made me go \"huh?\".\n\n  Huh yeah, sorry, that's totally spurious indeed.\n\n> But everything else looked pretty mechanical conversion, and in that sense\n> it is not that intrusive.\n> \n> Applying this to 'master' and then merging 'pu' shows that there are a few\n> topics that are cooking that would conflict with this change.  Merging\n> 'next' seems to go cleanly (I haven't checked the result), so it is not\n> too bad for me to carrry this at this moment, if we were not this close to\n> the rc freeze.  I dunno.\n\n  Well I can wait longer, I'd just like to see it merged in a not too\nfar future, because I have to check for new places that would need\nconversions at each reabase :) (though the init/setup_revisions\nfunctions changed their number of arguments so I _think_ I should miss\nnone each time but …).\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"71608","messageId":"7vwsoaid6z.fsf@gitster.siamese.dyndns.org","threadId":"12507","inReplyTo":"20080310084920.GA2798@artemis.madism.org","subject":"Re: [PATCH] revisions: refactor init_revisions and setup_revisions.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-10T19:36:20Z","receivedAt":"2008-03-10T19:36:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n>   We could of course have a .def member in the struct rev_info, and use\n> the one passed to setup_revisions then if it's still NULL, but it\n> doesn't really makes sense to me, and I don't really see a problem with\n> saying at init time that you'll default to \"HEAD\". Though if you really\n> dislike it that much, I squash a patch that does that on top of it.\n\nWell, it was not liking or disliking.  Although I thought \"default\" that\nsets a value to the default after the parser finds that the user did not\ngive anything (the approach you described in the above quoted paragraph)\nis a natural implementation, probably more so than what you did, I do not\nhave strong preference either way.\n\nWhen I see a change where I do not see a reason to, I get suspicious,\nwondering if I am missing some bigger reason (e.g. \"by moving it there\nthis and that would become much easier and cleaner, even though it now\nforces callers of cmd_log_init() to duplicate the default values\").  There\nmust be an obvious justification you had when you changed it, which I am\nnot seeing.  Hence that question.\n\n>> Applying this to 'master' and then merging 'pu' shows that there are a few\n>> topics that are cooking that would conflict with this change.  Merging\n>> 'next' seems to go cleanly (I haven't checked the result), so it is not\n>> too bad for me to carrry this at this moment, if we were not this close to\n>> the rc freeze.  I dunno.\n>\n>   Well I can wait longer, I'd just like to see it merged in a not too\n> far future, because I have to check for new places that would need\n> conversions at each reabase :)\n\nYeah, that burden can be shifted to me, in other words ;-)\n"},{"id":"71611","messageId":"20080310204407.GA30569@artemis.madism.org","threadId":"12507","inReplyTo":"7vwsoaid6z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] revisions: refactor init_revisions and setup_revisions.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-10T20:44:07Z","receivedAt":"2008-03-10T20:44:07Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Mar 10, 2008 at 07:36:20PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> >   We could of course have a .def member in the struct rev_info, and use\n> > the one passed to setup_revisions then if it's still NULL, but it\n> > doesn't really makes sense to me, and I don't really see a problem with\n> > saying at init time that you'll default to \"HEAD\". Though if you really\n> > dislike it that much, I squash a patch that does that on top of it.\n> \n> Well, it was not liking or disliking.  Although I thought \"default\" that\n> sets a value to the default after the parser finds that the user did not\n> give anything (the approach you described in the above quoted paragraph)\n> is a natural implementation, probably more so than what you did, I do not\n> have strong preference either way.\n\n  Well Okay, I'll let others comment, and will implement this way if\nmore are in favor of it than against then :)\n\n> >> Applying this to 'master' and then merging 'pu' shows that there are a few\n> >> topics that are cooking that would conflict with this change.  Merging\n> >> 'next' seems to go cleanly (I haven't checked the result), so it is not\n> >> too bad for me to carrry this at this moment, if we were not this close to\n> >> the rc freeze.  I dunno.\n> >\n> >   Well I can wait longer, I'd just like to see it merged in a not too\n> > far future, because I have to check for new places that would need\n> > conversions at each reabase :)\n> \n> Yeah, that burden can be shifted to me, in other words ;-)\n\n  Heh, fair enough. Well, I can wait a bit longer :)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"}]}