{"thread":{"id":"27151","subject":"[PATCH/RFC 0/3] teaching log's --glob=<glob> and friends to git shortlog","startedAt":"2011-04-21T10:22:41Z","lastAt":"2011-04-26T08:19:11Z","messageCount":11,"participants":["Jonathan Nieder","Michael J Gruber","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"166183","messageId":"20110421102241.GA16185@elie","threadId":"27151","inReplyTo":null,"subject":"[PATCH/RFC 0/3] teaching log's --glob=<glob> and friends to git shortlog","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-21T10:22:41Z","receivedAt":"2011-04-21T10:22:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nWhile looking over the setup_revisions code to work on an interesting\nrequest from Josh Triplett[1] (that happens to coincide with a\nlongstanding wish of my own), I got distracted by a few things.  The\nnominal subject of this series is to teach \"git shortlog\" to handle\ncommands like\n\n\tgit shortlog --glob=remotes/charon/notes/*\n\nPatch 1 makes the code clearer about something I had been confused\nby for a long time --- the --no-walk option invented for \"git show\"'s\nbenefit has global effect.  So tricks like[2]\n\n\tgit rev-list ^HEAD --no-walk commit1 commit2 ...\n\nto list branches not contained in HEAD will not work, alas.\n\nPatch 2 unindents the code a little.\n\nPatch 3 is the title piece --- it adds --glob=, --tag=, and so forth\nto the whitelist in handle_revision_opt so commands like \"git\nshortlog\" that parse revision listing options in two stages will queue\nthem correctly.\n\nWhile working on this I was struck by how close the setup_revisions\ncode has been to parseoptification all these years.  Maybe some\nweekend.\n\nThoughts and improvements welcome, of course.\n\nJonathan Nieder (3):\n  revisions: clarify handling of --no-walk and --do-walk\n  revisions: split out handle_revision_pseudo_opt function\n  revisions: allow --glob and friends in parse_options-enabled commands\n\n revision.c               |  138 ++++++++++++++++++++++++----------------------\n t/t6018-rev-list-glob.sh |   50 +++++++++++++++++\n 2 files changed, 122 insertions(+), 66 deletions(-)\n\n[1] http://bugs.debian.org/621601\n[2] http://thread.gmane.org/gmane.comp.version-control.git/138109/focus=138124\n"},{"id":"166184","messageId":"20110421103926.GA16260@elie","threadId":"27151","inReplyTo":"20110421102241.GA16185@elie","subject":"[WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-21T10:39:26Z","receivedAt":"2011-04-21T10:39:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As v1.6.0-rc2~42 (Allow \"non-option\" revision options in\nparse_option-enabled commands, 2008-07-31) explains, commands which\nuse parse_options() but also call setup_revisions() do their parsing\nin two stages:\n\n 1. first, they parse all options. Anything unknown goes to\n    parse_revision_opt() (which calls handle_revision_opt), which\n    may claim the option or say \"I don't recognize this\"\n\n 2. the non-option remainder goes to setup_revisions() to\n    actually get turned into revisions\n\nSome revision options, like --all and --not, are \"non-options\" in that\nthey must be parsed in order with their revision counterparts in\nsetup_revisions().  It would be nice if --no-walk and --do-walk fell\nin this category and set a flag only for revs coming after them on the\ncommand line, but they do not, so move parsing of --no-walk and\n--do-walk to the first \"global options\" stage for clarity.\n\n---\nWait, the above is not actually the full story.  If I do\n\n\tgit show maint..master\n\nthen this turns on walking automatically, to give the commit range\nmeaning.  Likewise\n\n\tgit log --no-walk maint..master\n\nwill, in fact walk, but\n\n\tgit log maint..master --no-walk\n\nwill not.  Which I should have understood from v1.6.0-rc2~42\n(2008-07-31) already.  Will think more; sorry for the nonsense.\n\n revision.c |   13 ++++---------\n 1 files changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0f38364..7b87bd0 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1177,7 +1177,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\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    !strcmp(arg, \"--bisect\"))\n \t{\n \t\tunkv[(*unkc)++] = arg;\n@@ -1334,6 +1333,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->unpacked = 1;\n \t} else if (!prefixcmp(arg, \"--unpacked=\")) {\n \t\tdie(\"--unpacked=<packfile> no longer supported.\");\n+\t} else if (!strcmp(arg, \"--no-walk\")) {\n+\t\trevs->no_walk = 1;\n+\t} else if (!strcmp(arg, \"--do-walk\")) {\n+\t\trevs->no_walk = 0;\n \t} else if (!strcmp(arg, \"-r\")) {\n \t\trevs->diff = 1;\n \t\tDIFF_OPT_SET(&revs->diffopt, RECURSIVE);\n@@ -1622,14 +1625,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\t\tflags ^= UNINTERESTING;\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-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--do-walk\")) {\n-\t\t\t\trevs->no_walk = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n \t\t\tif (!strcmp(arg, \"--stdin\")) {\n \t\t\t\tif (revs->disable_stdin) {\n \t\t\t\t\targv[left++] = arg;\n-- \n1.7.5.rc3\n"},{"id":"166185","messageId":"20110421104507.GB16260@elie","threadId":"27151","inReplyTo":"20110421102241.GA16185@elie","subject":"[PATCH 2/3] revisions: split out handle_revision_pseudo_opt function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-21T10:45:07Z","receivedAt":"2011-04-21T10:45:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As v1.6.0-rc2~42 (Allow \"non-option\" revision options in\nparse_option-enabled commands, 2008-07-31) explains, options handled\nby setup_revisions fall into two categories:\n\n 1. global options like --topo-order handled by parse_revision_opt,\n    which can take detached arguments and can be parsed in advance;\n\n 2. pseudo-options that must be parsed in order with their revision\n    counterparts, like --not and --all.\n\nThe global options are taken care of by handle_revision_opt; the\npseudo-options are currently in a deeply indented portion of\nsetup_revisions.  Give them their own function for easier reading.\n\nThe only goal is to make setup_revisions easier to read straight\nthrough.  No functional change intended.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nContinuing without patch 1.\n\n revision.c |  123 +++++++++++++++++++++++++++++-------------------------------\n 1 files changed, 59 insertions(+), 64 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0f38364..c9b1e32 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1526,6 +1526,59 @@ static void append_prune_data(const char ***prune_data, const char **av)\n \t*prune_data = prune;\n }\n \n+static int handle_revision_pseudo_opt(const char *submodule,\n+\t\t\t\tstruct rev_info *revs,\n+\t\t\t\tint argc, const char **argv, int *flags)\n+{\n+\tconst char *arg = argv[0];\n+\tconst char *optarg;\n+\tint argcount;\n+\n+\tif (!strcmp(arg, \"--all\")) {\n+\t\thandle_refs(submodule, revs, *flags, for_each_ref_submodule);\n+\t\thandle_refs(submodule, revs, *flags, head_ref_submodule);\n+\t} else if (!strcmp(arg, \"--branches\")) {\n+\t\thandle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);\n+\t} else if (!strcmp(arg, \"--bisect\")) {\n+\t\thandle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);\n+\t\thandle_refs(submodule, revs, *flags ^ UNINTERESTING, for_each_good_bisect_ref);\n+\t\trevs->bisect = 1;\n+\t} else if (!strcmp(arg, \"--tags\")) {\n+\t\thandle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);\n+\t} else if (!strcmp(arg, \"--remotes\")) {\n+\t\thandle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);\n+\t} else if ((argcount = parse_long_opt(\"glob\", argv, &optarg))) {\n+\t\tstruct all_refs_cb cb;\n+\t\tinit_all_refs_cb(&cb, revs, *flags);\n+\t\tfor_each_glob_ref(handle_one_ref, optarg, &cb);\n+\t\treturn argcount;\n+\t} else if (!prefixcmp(arg, \"--branches=\")) {\n+\t\tstruct all_refs_cb cb;\n+\t\tinit_all_refs_cb(&cb, revs, *flags);\n+\t\tfor_each_glob_ref_in(handle_one_ref, arg + 11, \"refs/heads/\", &cb);\n+\t} else if (!prefixcmp(arg, \"--tags=\")) {\n+\t\tstruct all_refs_cb cb;\n+\t\tinit_all_refs_cb(&cb, revs, *flags);\n+\t\tfor_each_glob_ref_in(handle_one_ref, arg + 7, \"refs/tags/\", &cb);\n+\t} else if (!prefixcmp(arg, \"--remotes=\")) {\n+\t\tstruct all_refs_cb cb;\n+\t\tinit_all_refs_cb(&cb, revs, *flags);\n+\t\tfor_each_glob_ref_in(handle_one_ref, arg + 10, \"refs/remotes/\", &cb);\n+\t} else if (!strcmp(arg, \"--reflog\")) {\n+\t\thandle_reflog(revs, *flags);\n+\t} else if (!strcmp(arg, \"--not\")) {\n+\t\t*flags ^= UNINTERESTING;\n+\t} else if (!strcmp(arg, \"--no-walk\")) {\n+\t\trevs->no_walk = 1;\n+\t} else if (!strcmp(arg, \"--do-walk\")) {\n+\t\trevs->no_walk = 0;\n+\t} else {\n+\t\treturn 0;\n+\t}\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@@ -1538,8 +1591,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tint i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0;\n \tconst char **prune_data = NULL;\n \tconst char *submodule = NULL;\n-\tconst char *optarg;\n-\tint argcount;\n \n \tif (opt)\n \t\tsubmodule = opt->submodule;\n@@ -1566,70 +1617,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tif (*arg == '-') {\n \t\t\tint opts;\n \n-\t\t\tif (!strcmp(arg, \"--all\")) {\n-\t\t\t\thandle_refs(submodule, revs, flags, for_each_ref_submodule);\n-\t\t\t\thandle_refs(submodule, revs, flags, head_ref_submodule);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--branches\")) {\n-\t\t\t\thandle_refs(submodule, revs, flags, for_each_branch_ref_submodule);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--bisect\")) {\n-\t\t\t\thandle_refs(submodule, revs, flags, for_each_bad_bisect_ref);\n-\t\t\t\thandle_refs(submodule, revs, flags ^ UNINTERESTING, for_each_good_bisect_ref);\n-\t\t\t\trevs->bisect = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--tags\")) {\n-\t\t\t\thandle_refs(submodule, revs, flags, for_each_tag_ref_submodule);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--remotes\")) {\n-\t\t\t\thandle_refs(submodule, revs, flags, for_each_remote_ref_submodule);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif ((argcount = parse_long_opt(\"glob\", argv + i, &optarg))) {\n-\t\t\t\tstruct all_refs_cb cb;\n-\t\t\t\ti += argcount - 1;\n-\t\t\t\tinit_all_refs_cb(&cb, revs, flags);\n-\t\t\t\tfor_each_glob_ref(handle_one_ref, optarg, &cb);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--branches=\")) {\n-\t\t\t\tstruct all_refs_cb cb;\n-\t\t\t\tinit_all_refs_cb(&cb, revs, flags);\n-\t\t\t\tfor_each_glob_ref_in(handle_one_ref, arg + 11, \"refs/heads/\", &cb);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--tags=\")) {\n-\t\t\t\tstruct all_refs_cb cb;\n-\t\t\t\tinit_all_refs_cb(&cb, revs, flags);\n-\t\t\t\tfor_each_glob_ref_in(handle_one_ref, arg + 7, \"refs/tags/\", &cb);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!prefixcmp(arg, \"--remotes=\")) {\n-\t\t\t\tstruct all_refs_cb cb;\n-\t\t\t\tinit_all_refs_cb(&cb, revs, flags);\n-\t\t\t\tfor_each_glob_ref_in(handle_one_ref, arg + 10, \"refs/remotes/\", &cb);\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, \"--not\")) {\n-\t\t\t\tflags ^= UNINTERESTING;\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-\t\t\t}\n-\t\t\tif (!strcmp(arg, \"--do-walk\")) {\n-\t\t\t\trevs->no_walk = 0;\n+\t\t\topts = handle_revision_pseudo_opt(submodule,\n+\t\t\t\t\t\trevs, argc - i, argv + i,\n+\t\t\t\t\t\t&flags);\n+\t\t\tif (opts > 0) {\n+\t\t\t\ti += opts - 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\n \t\t\tif (!strcmp(arg, \"--stdin\")) {\n \t\t\t\tif (revs->disable_stdin) {\n \t\t\t\t\targv[left++] = arg;\n-- \n1.7.5.rc3\n"},{"id":"166187","messageId":"20110421104823.GC16260@elie","threadId":"27151","inReplyTo":"20110421102241.GA16185@elie","subject":"[PATCH 3/3] revisions: allow --glob and friends in parse_options-enabled commands","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-21T10:48:24Z","receivedAt":"2011-04-21T10:48:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As v1.6.0-rc2~42 (2008-07-31) explains, even pseudo-options like --not\nand --glob that need to be parsed in order with revisions should be\nmarked handled by handle_revision_opt to avoid an error when\nparse_revision_opt callers like \"git shortlog\" encounter them.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThanks for reading.  I hope despite the mess it was not too\nunpleasant.\n\n revision.c               |   14 ++++++++++++-\n t/t6018-rev-list-glob.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 63 insertions(+), 1 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex c9b1e32..2389764 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1178,7 +1178,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\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    !strcmp(arg, \"--bisect\"))\n+\t    !strcmp(arg, \"--bisect\") || !prefixcmp(arg, \"--glob=\") ||\n+\t    !prefixcmp(arg, \"--branches=\") || !prefixcmp(arg, \"--tags=\") ||\n+\t    !prefixcmp(arg, \"--remotes=\"))\n \t{\n \t\tunkv[(*unkc)++] = arg;\n \t\treturn 1;\n@@ -1534,6 +1536,16 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \tconst char *optarg;\n \tint argcount;\n \n+\t/*\n+\t * NOTE!\n+\t *\n+\t * Commands like \"git shortlog\" will not accept the options below\n+\t * unless parse_revision_opt queues them (as opposed to erroring\n+\t * out).\n+\t *\n+\t * When implementing your new pseudo-option, remember to\n+\t * register it in the list at the top of handle_revision_opt.\n+\t */\n \tif (!strcmp(arg, \"--all\")) {\n \t\thandle_refs(submodule, revs, *flags, for_each_ref_submodule);\n \t\thandle_refs(submodule, revs, *flags, head_ref_submodule);\ndiff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh\nindex fb8291c..f00cebf 100755\n--- a/t/t6018-rev-list-glob.sh\n+++ b/t/t6018-rev-list-glob.sh\n@@ -69,6 +69,18 @@ test_expect_success 'rev-parse --glob=heads/subspace' '\n \n '\n \n+test_expect_failure 'rev-parse accepts --glob as detached option' '\n+\n+\tcompare rev-parse \"subspace/one subspace/two\" \"--glob heads/subspace\"\n+\n+'\n+\n+test_expect_failure 'rev-parse is not confused by option-like glob' '\n+\n+\tcompare rev-parse \"master\" \"--glob --symbolic master\"\n+\n+'\n+\n test_expect_success 'rev-parse --branches=subspace/*' '\n \n \tcompare rev-parse \"subspace/one subspace/two\" \"--branches=subspace/*\"\n@@ -129,6 +141,12 @@ test_expect_success 'rev-list --glob refs/heads/subspace/*' '\n \n '\n \n+test_expect_success 'rev-list not confused by option-like --glob arg' '\n+\n+\tcompare rev-list \"master\" \"--glob -0 master\"\n+\n+'\n+\n test_expect_success 'rev-list --glob=heads/subspace/*' '\n \n \tcompare rev-list \"subspace/one subspace/two\" \"--glob=heads/subspace/*\"\n@@ -213,4 +231,36 @@ test_expect_success 'rev-list --remotes=foo' '\n \n '\n \n+test_expect_success 'shortlog accepts --glob/--tags/--remotes' '\n+\n+\tcompare shortlog \"subspace/one subspace/two\" --branches=subspace &&\n+\tcompare shortlog \\\n+\t  \"master subspace-x someref other/three subspace/one subspace/two\" \\\n+\t  --branches &&\n+\tcompare shortlog master \"--glob=heads/someref/* master\" &&\n+\tcompare shortlog \"subspace/one subspace/two other/three\" \\\n+\t  \"--glob=heads/subspace/* --glob=heads/other/*\" &&\n+\tcompare shortlog \\\n+\t  \"master other/three someref subspace-x subspace/one subspace/two\" \\\n+\t  \"--glob=heads/*\" &&\n+\tcompare shortlog foo/bar --tags=foo &&\n+\tcompare shortlog foo/bar --tags &&\n+\tcompare shortlog foo/baz --remotes=foo\n+\n+'\n+\n+test_expect_failure 'shortlog accepts --glob as detached option' '\n+\n+\tcompare shortlog \\\n+\t  \"master other/three someref subspace-x subspace/one subspace/two\" \\\n+\t  \"--glob heads/*\"\n+\n+'\n+\n+test_expect_failure 'shortlog --glob is not confused by option-like argument' '\n+\n+\tcompare shortlog master \"--glob -e master\"\n+\n+'\n+\n test_done\n-- \n1.7.5.rc3\n"},{"id":"166190","messageId":"4DB02B24.4030503@drmicha.warpmail.net","threadId":"27151","inReplyTo":"20110421103926.GA16260@elie","subject":"Re: [WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-21T13:03:32Z","receivedAt":"2011-04-21T13:03:32Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jonathan Nieder venit, vidit, dixit 21.04.2011 12:39:\n> As v1.6.0-rc2~42 (Allow \"non-option\" revision options in\n> parse_option-enabled commands, 2008-07-31) explains, commands which\n> use parse_options() but also call setup_revisions() do their parsing\n> in two stages:\n> \n>  1. first, they parse all options. Anything unknown goes to\n>     parse_revision_opt() (which calls handle_revision_opt), which\n>     may claim the option or say \"I don't recognize this\"\n> \n>  2. the non-option remainder goes to setup_revisions() to\n>     actually get turned into revisions\n> \n> Some revision options, like --all and --not, are \"non-options\" in that\n> they must be parsed in order with their revision counterparts in\n> setup_revisions().  It would be nice if --no-walk and --do-walk fell\n> in this category and set a flag only for revs coming after them on the\n> command line, but they do not, so move parsing of --no-walk and\n> --do-walk to the first \"global options\" stage for clarity.\n> \n> ---\n> Wait, the above is not actually the full story.  If I do\n> \n> \tgit show maint..master\n> \n> then this turns on walking automatically, to give the commit range\n> meaning.  Likewise\n> \n> \tgit log --no-walk maint..master\n> \n> will, in fact walk, but\n> \n> \tgit log maint..master --no-walk\n> \n> will not.  Which I should have understood from v1.6.0-rc2~42\n> (2008-07-31) already.  Will think more; sorry for the nonsense.\n\nThis is not unrelated to the tip of gitster/mg/show-without-prune, i.e.\n\n0c738b6 (builtin/show: do not prune by pathspec, 2011-04-01)\n\nSee http://permalink.gmane.org/gmane.comp.version-control.git/170461\n\nWe should rethink the ui balance between deviating from the usual log\noption processing and the usefulness here.\n\nMichael\n"},{"id":"166196","messageId":"7vtydry038.fsf@alter.siamese.dyndns.org","threadId":"27151","inReplyTo":"20110421102241.GA16185@elie","subject":"Re: [PATCH/RFC 0/3] teaching log's --glob=<glob> and friends to git shortlog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-21T17:40:27Z","receivedAt":"2011-04-21T17:40:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> \tgit rev-list ^HEAD --no-walk commit1 commit2 ...\n\nThat should be \"git rev-list --no-walk ^HEAD commit1 commit2 ...\" from the\nsyntactical point of view to begin with, but more importantly, what does\nit mean to ask excluding commits reachable from HEAD without walking?\n\nAs ^HEAD is inherently about walking the history to paint the part of DAG\nthat are not interesting because the nodes in there are reachable from\nthere, the request that command line tries to make does not make much\nsense.\n\nAs I said in\n\n http://thread.gmane.org/gmane.comp.version-control.git/170457/focus=170629\n\nthe \"--no-walk\" hack happens to work by accident, and we would need to\nrethink the way it operates and revamp it.\n"},{"id":"166205","messageId":"20110421211613.GA18418@elie","threadId":"27151","inReplyTo":"7vtydry038.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 0/3] teaching log's --glob=<glob> and friends to git shortlog","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-21T21:16:53Z","receivedAt":"2011-04-21T21:16:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> \tgit rev-list ^HEAD --no-walk commit1 commit2 ...\n>\n> That should be \"git rev-list --no-walk ^HEAD commit1 commit2 ...\" from the\n> syntactical point of view to begin with, but more importantly, what does\n> it mean to ask excluding commits reachable from HEAD without walking?\n\nThis framing of the question is based on --no-walk being a global\nflag.  It is possible to imagine it instead being a positional\nargument like --not, without invasive changes to the code structure,\nas in the patch below.\n\n> As ^HEAD is inherently about walking the history to paint the part of DAG\n> that are not interesting because the nodes in there are reachable from\n> there, the request that command line tries to make does not make much\n> sense.\n\nA separate question is whether paying attention to the position of\n--no-walk in the command line was a good idea.  Let's see what it\nwould take to reconcile my belief that \"no, it isn't\" with current\npractice.\n\nThe idea would be to introduce a new option --behave-like-show with\nthe following semantics:\n\n. \"git log --behave-like-show foo..bar\" is a synonym for \"git log foo..bar\"\n. \"git log --behave-like-show -5 foo\" is a synonym for \"git log -5 foo\"\n. \"git log --behave-like-show --do-walk foo\" is a synonym for \"git log foo\"\n. \"git log --behave-like-show foo\" is a synonym for \"git log --no-walk foo\"\n\nThen we could remove the existing magic that turns \"git log --no-walk foo..bar\"\nto \"git log foo..bar\" and error out since the operator has requested\nsomething undoable (\"--no-walk is incompatible with negative revisions\").\n\nMore precisely, it could work like so:\n\n - --no-walk/--do-walk becomes a tristate (walk, don't walk, default)\n - --behave-like-show state is another tristate (seen ^foo or -5,\n   using --behave-like-show but not seen ^foo or -5, neither)\n\nThe rules:\n\n - if we saw --no-walk (and it was not cancelled by a later --do-walk)\n   along with ^foo, error out.\n - otherwise, if we saw --do-walk (and it was not cancelled by a later\n   --no-walk), walk.\n - otherwise, if we saw --no-walk, don't walk.  If we saw ^foo, walk.\n - by default, walk if and only if we didn't see --behave-like-show.\n\nSo the usual \"git show\" usage patterns would work, and the position of\n--no-walk (except in that it can be cancelled by --do-walk) wouldn't\nmatter.\n\n> As I said in\n>\n>  http://thread.gmane.org/gmane.comp.version-control.git/170457/focus=170629\n>\n> the \"--no-walk\" hack happens to work by accident, and we would need to\n> rethink the way it operates and revamp it.\n\nYes, [1] is interesting (though a bit broader in scope).\n\nThe promised nonsense patch implementing a NO_WALK commit flag (i.e.,\nthe approach the patch upthread was meant to clarify we are not using)\nfollows.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/170457/focus=170839\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n\nPlease feel free to play around with it but do not consider\nit for inclusion. :)\n---\n revision.c |   37 ++++++++++++++++++++++++++++---------\n revision.h |    4 +++-\n 2 files changed, 31 insertions(+), 10 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0f38364..dae16da 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -133,8 +133,10 @@ void mark_parents_uninteresting(struct commit *commit)\n \n static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)\n {\n-\tif (revs->no_walk && (obj->flags & UNINTERESTING))\n+\tif (obj->flags & UNINTERESTING) {\n+\t\tobj->flags &= ~NO_WALK;\n \t\trevs->no_walk = 0;\n+\t}\n \tif (revs->reflog_info && obj->type == OBJ_COMMIT) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\tint len = interpret_branch_name(name, &buf);\n@@ -166,6 +168,10 @@ void add_head_to_pending(struct rev_info *revs)\n \tobj = parse_object(sha1);\n \tif (!obj)\n \t\treturn;\n+\tif (revs->no_walk)\n+\t\tobj->flags |= NO_WALK;\n+\telse\n+\t\tobj->flags &= ~NO_WALK;\n \tadd_pending_object(revs, obj, \"HEAD\");\n }\n \n@@ -506,7 +512,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,\n \t */\n \ttry_to_simplify_commit(revs, commit);\n \n-\tif (revs->no_walk)\n+\tif ((commit->object.flags & NO_WALK) && !revs->force_walk)\n \t\treturn 0;\n \n \tleft_flag = (commit->object.flags & SYMMETRIC_LEFT);\n@@ -1062,6 +1068,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \t\t\t} else\n \t\t\t\ta->object.flags |= flags_exclude;\n \t\t\tb->object.flags |= flags;\n+\t\t\tb->object.flags &= ~NO_WALK;\n \t\t\tadd_pending_object(revs, &a->object, this);\n \t\t\tadd_pending_object(revs, &b->object, next);\n \t\t\treturn 0;\n@@ -1128,6 +1135,7 @@ static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune\n {\n \tstruct strbuf sb;\n \tint seen_dashdash = 0;\n+\tint flags = 0;\n \n \tstrbuf_init(&sb, 1000);\n \twhile (strbuf_getwholeline(&sb, stdin, '\\n') != EOF) {\n@@ -1143,7 +1151,11 @@ static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune\n \t\t\t}\n \t\t\tdie(\"options not supported in --stdin mode\");\n \t\t}\n-\t\tif (handle_revision_arg(sb.buf, revs, 0, 1))\n+\t\tif (revs->no_walk)\n+\t\t\tflags |= NO_WALK;\n+\t\telse\n+\t\t\tflags &= ~NO_WALK;\n+\t\tif (handle_revision_arg(sb.buf, revs, flags, 1))\n \t\t\tdie(\"bad revision '%s'\", sb.buf);\n \t}\n \tif (seen_dashdash)\n@@ -1186,7 +1198,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n \t\trevs->max_count = atoi(optarg);\n-\t\trevs->no_walk = 0;\n+\t\trevs->force_walk = 1;\n \t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n \t\trevs->skip_count = atoi(optarg);\n@@ -1194,16 +1206,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n \t/* accept -<digit>, like traditional \"head\" */\n \t\trevs->max_count = atoi(arg + 1);\n-\t\trevs->no_walk = 0;\n+\t\trevs->force_walk = 1;\n \t} else 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\trevs->no_walk = 0;\n+\t\trevs->force_walk = 1;\n \t\treturn 2;\n \t} else if (!prefixcmp(arg, \"-n\")) {\n \t\trevs->max_count = atoi(arg + 2);\n-\t\trevs->no_walk = 0;\n+\t\trevs->force_walk = 1;\n \t} else if ((argcount = parse_long_opt(\"max-age\", argv, &optarg))) {\n \t\trevs->max_age = atoi(optarg);\n \t\treturn argcount;\n@@ -1563,6 +1575,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tread_from_stdin = 0;\n \tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tif (revs->no_walk)\n+\t\t\tflags |= NO_WALK;\n+\t\telse\n+\t\t\tflags &= ~NO_WALK;\n+\n \t\tif (*arg == '-') {\n \t\t\tint opts;\n \n@@ -1688,6 +1705,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\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\tif (revs->no_walk)\n+\t\t\tobject->flags |= NO_WALK;\n+\t\telse\n+\t\t\tobject->flags &= ~NO_WALK;\n \t\tadd_pending_object_with_mode(revs, object, revs->def, mode);\n \t}\n \n@@ -1951,8 +1972,6 @@ int prepare_revision_walk(struct rev_info *revs)\n \t}\n \tfree(list);\n \n-\tif (revs->no_walk)\n-\t\treturn 0;\n \tif (revs->limited)\n \t\tif (limit_list(revs) < 0)\n \t\t\treturn -1;\ndiff --git a/revision.h b/revision.h\nindex 9fd8f30..60f00ff 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -15,7 +15,8 @@\n #define ADDED\t\t(1u<<7)\t/* Parents already parsed and added? */\n #define SYMMETRIC_LEFT\t(1u<<8)\n #define PATCHSAME\t(1u<<9)\n-#define ALL_REV_FLAGS\t((1u<<10)-1)\n+#define NO_WALK\t\t(1u<<10)\n+#define ALL_REV_FLAGS\t((1u<<11)-1)\n \n #define DECORATE_SHORT_REFS\t1\n #define DECORATE_FULL_REFS\t2\n@@ -42,6 +43,7 @@ struct rev_info {\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n \t\t\tno_walk:1,\n+\t\t\tforce_walk:1,\n \t\t\tshow_all:1,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\n-- \n1.7.5.rc3\n"},{"id":"166207","messageId":"20110421213014.GB18418@elie","threadId":"27151","inReplyTo":"4DB02B24.4030503@drmicha.warpmail.net","subject":"Re: [WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-21T21:30:14Z","receivedAt":"2011-04-21T21:30:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael J Gruber wrote:\n\n> This is not unrelated to the tip of gitster/mg/show-without-prune, i.e.\n>\n> 0c738b6 (builtin/show: do not prune by pathspec, 2011-04-01)\n\nTrue.\n\n> We should rethink the ui balance between deviating from the usual log\n> option processing and the usefulness here.\n\nCould you expand on that?  I think --no-walk being a positional\nargument is just an ugly consequence of the code that permits\n\n\tgit show -5 master\n\tgit show maint..\n\tgit show --do-walk master\n\nwhile making sure\n\n\tgit show master\n\tgit show --do-walk --no-walk master\n\nstill show only one commit.  I'm not convinced it's meant to be useful\nbeyond that.\n\n(Maybe \"git log --no-walk maint..HEAD\" should be an error because\nmeaningless, while \"git rev-list --no-walk maint..HEAD\" should be\naccepted so people can continue write their own \"git show\"-like\ncommands.  Unfortunately the popular idiom of using git log\n--format=... instead of\n\n\tgit rev-list ... |\n\tgit diff-tree -s --format=... --stdin\n\nin scripts makes this porcelain-plumbing distinction more fuzzy than I\nwould like.)\n"},{"id":"166277","messageId":"4DB40941.5030903@drmicha.warpmail.net","threadId":"27151","inReplyTo":"20110421213014.GB18418@elie","subject":"Re: [WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-24T11:28:01Z","receivedAt":"2011-04-24T11:28:01Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jonathan Nieder venit, vidit, dixit 21.04.2011 23:30:\n> Michael J Gruber wrote:\n> \n>> This is not unrelated to the tip of gitster/mg/show-without-prune, i.e.\n>>\n>> 0c738b6 (builtin/show: do not prune by pathspec, 2011-04-01)\n> \n> True.\n> \n>> We should rethink the ui balance between deviating from the usual log\n>> option processing and the usefulness here.\n> \n> Could you expand on that?  I think --no-walk being a positional\n> argument is just an ugly consequence of the code that permits\n> \n\nSorry for being fuzzy. What I meant was: There is the systematic\napproach which gives the user exactly what he asks for, i.e.: log,\nrev-list default to walking, show to not walking (and rev-parse of\ncourse), and when a user feeds a range it may or may may not make sense.\nSimilarly, \"git show commit -- path\" returns nothing at all if path is\nnot touched by commit because the commit gets pruned.\n\nWe don't do the systematic approach now. In some situations, some\ncommands switch on the walker automatically (I think \"show A..B\") to\nmake things more useful (to most users) but less systematic, even less\npredictable if you don't know these deviations/exceptions. I've\nsuggested such a \"usefulness exception\" myself (don't prune commits by\npath for \"show\").\n\nThe strict, systematic approach produces some command/argument\ncombinations which are not useful or rarely useful.\n\nThe \"helpful\" approach produces special casing which may make things\nconfusing if there's too much of it.\n\nFinding the right balance is probably more difficult than being\nradically systematic...\n\nMichael\n"},{"id":"166293","messageId":"20110425002124.GB31168@elie","threadId":"27151","inReplyTo":"4DB40941.5030903@drmicha.warpmail.net","subject":"Re: [WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-25T00:21:24Z","receivedAt":"2011-04-25T00:21:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael J Gruber wrote:\n\n> We don't do the systematic approach now. In some situations, some\n> commands switch on the walker automatically (I think \"show A..B\") to\n> make things more useful (to most users) but less systematic, even less\n> predictable if you don't know these deviations/exceptions. I've\n> suggested such a \"usefulness exception\" myself (don't prune commits by\n> path for \"show\").\n\nAh.  To be clearer about the present state:\n\n - cmd_show reimplements much of get_revision, to work around the\n   revision walking machinery's lack of callbacks to print tags,\n   blobs, and so on.\n\n - ^A means \"--do-walk ^A\", and A..B means \"--do-walk ^A B\".  This\n   holds in rev-list, log, show, etc --- they all share the code that\n   does this.\n\n - Similarly, -5 means \"--do-walk -5\".\n\n - rev-parse shares a revision parser (get_sha1) with rev-list, but it\n   doesn't share an option parser (alas).\n\nI personally kind of like the \"don't prune commits by path with\n--no-walk\" idea.  Not sure what happened to that.\n\n> The strict, systematic approach produces some command/argument\n> combinations which are not useful or rarely useful.\n\nWell, --no-walk foo..bar would be nonsense regardless of whether \"git\nshow\" did the equivalent of \"git rev-list --no-walk ...\".  And when\nthe user supplies nonsense, why should she expect us not to give\nnonsense back?  So how about something along these lines?\n\nNeeds documentation and tests.\n\n-- >8 --\nSubject: revisions: treat \"log A..B --no-walk\" as a synonym for \"--no-walk A..B\"\n\nWhen git learned to treat \"git show A..B\" as \"git show --do-walk\nA..B\", it was only an accident that the implied --do-walk had that\nposition.\n\nSure, it meant that one could write \"git show A..B --no-walk\" to get\nthe ancient behavior of showing \"B\" unless the two commits are the\nsame, but that is not a convenience --- on the contrary, it's a\nconfusing wart that has required effort to preserve ever since (see\nv1.6.0-rc2~42, 2008-07-31, for example).  So let's simplify the\nsemantics.  After this patch:\n\nAttempts to show A..B or show -5 _always_ walk.  \"git log A..B\n--no-walk\" simply doesn't make sense, so we assume the A..B came from\nthe end-user who wanted to override a script's suggestion not to walk.\n\nThe effect of --no-walk is the same everywhere, except that later\n--do-walk overrides early --no-walk.\n\nThere is no longer a way to limit the number of commits listed to 5\nwithout walking or to provide a negative revision without walking.\nIf someone wants that, we can introduce a new option instead of\ntacking it onto the hack that supports \"git show\".\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n revision.c |   15 +++++++++------\n revision.h |    1 +\n 2 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 0f38364..0996c6f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -133,8 +133,8 @@ void mark_parents_uninteresting(struct commit *commit)\n \n static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)\n {\n-\tif (revs->no_walk && (obj->flags & UNINTERESTING))\n-\t\trevs->no_walk = 0;\n+\tif (obj->flags & UNINTERESTING)\n+\t\trevs->implied_walk = 1;\n \tif (revs->reflog_info && obj->type == OBJ_COMMIT) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\tint len = interpret_branch_name(name, &buf);\n@@ -1186,7 +1186,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \n \tif ((argcount = parse_long_opt(\"max-count\", argv, &optarg))) {\n \t\trevs->max_count = atoi(optarg);\n-\t\trevs->no_walk = 0;\n+\t\trevs->implied_walk = 1;\n \t\treturn argcount;\n \t} else if ((argcount = parse_long_opt(\"skip\", argv, &optarg))) {\n \t\trevs->skip_count = atoi(optarg);\n@@ -1194,16 +1194,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if ((*arg == '-') && isdigit(arg[1])) {\n \t/* accept -<digit>, like traditional \"head\" */\n \t\trevs->max_count = atoi(arg + 1);\n-\t\trevs->no_walk = 0;\n+\t\trevs->implied_walk = 1;\n \t} else 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\trevs->no_walk = 0;\n+\t\trevs->implied_walk = 1;\n \t\treturn 2;\n \t} else if (!prefixcmp(arg, \"-n\")) {\n \t\trevs->max_count = atoi(arg + 2);\n-\t\trevs->no_walk = 0;\n+\t\trevs->implied_walk = 1;\n \t} else if ((argcount = parse_long_opt(\"max-age\", argv, &optarg))) {\n \t\trevs->max_age = atoi(optarg);\n \t\treturn argcount;\n@@ -1691,6 +1691,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tadd_pending_object_with_mode(revs, object, revs->def, mode);\n \t}\n \n+\tif (revs->implied_walk)\n+\t\trevs->no_walk = 0;\n+\n \t/* Did the user ask for any diff output? Run the diff! */\n \tif (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)\n \t\trevs->diff = 1;\ndiff --git a/revision.h b/revision.h\nindex 9fd8f30..c18d2c1 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -42,6 +42,7 @@ struct rev_info {\n \tunsigned int\tdense:1,\n \t\t\tprune:1,\n \t\t\tno_walk:1,\n+\t\t\timplied_walk:1,\n \t\t\tshow_all:1,\n \t\t\tremove_empty_trees:1,\n \t\t\tsimplify_history:1,\n-- \n1.7.5.rc3\n"},{"id":"166361","messageId":"4DB67FFF.1060709@drmicha.warpmail.net","threadId":"27151","inReplyTo":"20110425002124.GB31168@elie","subject":"Re: [WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-26T08:19:11Z","receivedAt":"2011-04-26T08:19:11Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jonathan Nieder venit, vidit, dixit 25.04.2011 02:21:\n> Michael J Gruber wrote:\n> \n>> We don't do the systematic approach now. In some situations, some\n>> commands switch on the walker automatically (I think \"show A..B\") to\n>> make things more useful (to most users) but less systematic, even less\n>> predictable if you don't know these deviations/exceptions. I've\n>> suggested such a \"usefulness exception\" myself (don't prune commits by\n>> path for \"show\").\n> \n> Ah.  To be clearer about the present state:\n> \n>  - cmd_show reimplements much of get_revision, to work around the\n>    revision walking machinery's lack of callbacks to print tags,\n>    blobs, and so on.\n> \n>  - ^A means \"--do-walk ^A\", and A..B means \"--do-walk ^A B\".  This\n>    holds in rev-list, log, show, etc --- they all share the code that\n>    does this.\n> \n>  - Similarly, -5 means \"--do-walk -5\".\n> \n>  - rev-parse shares a revision parser (get_sha1) with rev-list, but it\n>    doesn't share an option parser (alas).\n> \n> I personally kind of like the \"don't prune commits by path with\n> --no-walk\" idea.  Not sure what happened to that.\n\nIt got dropped after the switch to the new cycle. The question/problem\nwas whether some people used \"git show A B C -- path\" as a kind of\ncommit filter, selecting only commits which touch path (i.e. prune\ncommits by path). I would claim it's the wrong command to do that and\nwas never documented anyways...\n\nSo, my choice would be \"do not prune commits unless --do-walk was given\"\n(where \"was given\" may include \"was switched on by our range=>walk logic\").\n\nAnother choice would be \"do not prune if there is only one rev argument\nand no --do-walk\".\n\n> Ah.  To be clearer about the present state:\n> \n>  - cmd_show reimplements much of get_revision, to work around the\n>    revision walking machinery's lack of callbacks to print tags,\n>    blobs, and so on.\n> \n\nYou mean, other than pretty formats... I think our code does a couple of\nthings by hand which could be done with a pretty format. Though a\ncallback may be more efficient, unless our formats are pre-parsed.\n\nMichael\n"}]}