{"thread":{"id":"49253","subject":"[RFC] revision: Don't let ^ cancel out the default ","startedAt":"2018-08-29T20:05:59Z","lastAt":"2018-08-29T23:30:30Z","messageCount":3,"participants":["Andreas Gruenbacher","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"356872","messageId":"20180829200553.19379-1-agruenba@redhat.com","threadId":"49253","inReplyTo":null,"subject":"[RFC] revision: Don't let ^ cancel out the default ","fromName":"Andreas Gruenbacher","fromEmail":"agruenba@redhat.com","sentAt":"2018-08-29T20:05:53Z","receivedAt":"2018-08-29T20:05:59Z","isPatch":false,"sender":{"key":"agruenba@redhat.com","avatar":null},"body":"Some commands like 'log' default to HEAD if no other revisions are\nspecified on the command line or otherwise. Unfortunately, excludes\n(^) cancel out that default, so when a command only excludes\nrevisions (e.g., 'git log ^origin/master'), the command will not produce\nany result.\n\nIf all the specified revisions are excludes, it seems more useful to\nstick with the default revision instead.\n\nThis makes writing wrappers that exclude certain revisions much easier:\nfor example, a simple alias l='git log ^origin/master' will show the\nrevisions between origin/master and HEAD by default, and 'l foo' will\nshow the revisions between origin/master and foo, as you would usually\nexpect.\n\nSigned-off-by: Andreas Gruenbacher \n---\n revision.c | 18 ++++++++++++++----\n t/t4202-log.sh | 6 ++++++\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex de4dce600..c2c51bd5d 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1158,6 +1158,18 @@ static void add_rev_cmdline(struct rev_info *revs,\n \tinfo->nr++;\n }\n \n+static int has_interesting_revisions(struct rev_info *revs)\n+{\n+\tstruct rev_cmdline_info *info = &revs->cmdline;\n+\tunsigned int n;\n+\n+\tfor (n = 0; n < info->nr; n++) {\n+\t\tif (!(info->rev[n].flags & UNINTERESTING))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static void add_rev_cmdline_list(struct rev_info *revs,\n \t\t\t\t struct commit_list *commit_list,\n \t\t\t\t int whence,\n@@ -2318,7 +2330,7 @@ static void NORETURN diagnose_missing_default(const char *def)\n */\n int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *opt)\n {\n-\tint i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0, revarg_opt;\n+\tint i, flags, left, seen_dashdash, read_from_stdin, revarg_opt;\n \tstruct argv_array prune_data = ARGV_ARRAY_INIT;\n \tconst char *submodule = NULL;\n \n@@ -2401,8 +2413,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\targv_array_pushv(&prune_data, argv + i);\n \t\t\tbreak;\n \t\t}\n-\t\telse\n-\t\t\tgot_rev_arg = 1;\n \t}\n \n \tif (prune_data.argc) {\n@@ -2431,7 +2441,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\topt->tweak(revs, opt);\n \tif (revs->show_merge)\n \t\tprepare_show_merge(revs);\n-\tif (revs->def && !revs->pending.nr && !revs->rev_input_given && !got_rev_arg) {\n+\tif (revs->def && !revs->rev_input_given && !has_interesting_revisions(revs)) {\n \t\tstruct object_id oid;\n \t\tstruct object *object;\n \t\tstruct object_context oc;\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 153a50615..e6670859c 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -213,6 +213,12 @@ test_expect_success 'git show leaves list of commits as given' '\n \ttest_cmp expect actual\n '\n \n+printf \"sixth\\nfifth\\n\" > expect\n+test_expect_success '^' '\n+\tgit log --pretty=\"tformat:%s\" ^HEAD~2 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'setup case sensitivity tests' '\n \techo case >one &&\n \ttest_tick &&\n-- \n2.17.1\n\n"},{"id":"356909","messageId":"xmqqin3s94vf.fsf@gitster-ct.c.googlers.com","threadId":"49253","inReplyTo":"20180829200553.19379-1-agruenba@redhat.com","subject":"Re: [RFC] revision: Don't let ^ cancel out the default ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-29T21:03:34Z","receivedAt":"2018-08-29T22:37:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Gruenbacher writes:\n\n> Some commands like 'log' default to HEAD if no other revisions are\n> specified on the command line or otherwise. Unfortunately, excludes\n> (^) cancel out that default, so when a command only excludes\n> revisions (e.g., 'git log ^origin/master'), the command will not produce\n> any result.\n>\n> If all the specified revisions are excludes, it seems more useful to\n> stick with the default revision instead.\n>\n> This makes writing wrappers that exclude certain revisions much easier:\n> for example, a simple alias l='git log ^origin/master' will show the\n> revisions between origin/master and HEAD by default, and 'l foo' will\n> show the revisions between origin/master and foo, as you would usually\n> expect.\n\nThat is a _huge_ departure from the behaviour established for the\npast 10 years, but it would certainly be more useful.\n\nAs long as we can prove that that a command line with only negative\nrevs is absolutely useless, the backward incompatibility may be OK\nto swallow, especially for commands like \"git log\" that implicitly\nuse \"--default HEAD\", as they are meant for human consumption, and\nnot for scripts.\n\nI am not offhand 100% sure that a rev list with only negative revs\nis totally useless, though.\n"},{"id":"356915","messageId":"20180829233026.GA15005@sigill.intra.peff.net","threadId":"49253","inReplyTo":"xmqqin3s94vf.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC] revision: Don't let ^ cancel out the default ","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-29T23:30:26Z","receivedAt":"2018-08-29T23:30:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 29, 2018 at 02:03:34PM -0700, Junio C Hamano wrote:\n\n> Andreas Gruenbacher writes:\n> \n> > Some commands like 'log' default to HEAD if no other revisions are\n> > specified on the command line or otherwise. Unfortunately, excludes\n> > (^) cancel out that default, so when a command only excludes\n> > revisions (e.g., 'git log ^origin/master'), the command will not produce\n> > any result.\n> >\n> > If all the specified revisions are excludes, it seems more useful to\n> > stick with the default revision instead.\n> >\n> > This makes writing wrappers that exclude certain revisions much easier:\n> > for example, a simple alias l='git log ^origin/master' will show the\n> > revisions between origin/master and HEAD by default, and 'l foo' will\n> > show the revisions between origin/master and foo, as you would usually\n> > expect.\n> \n> That is a _huge_ departure from the behaviour established for the\n> past 10 years, but it would certainly be more useful.\n> \n> As long as we can prove that that a command line with only negative\n> revs is absolutely useless, the backward incompatibility may be OK\n> to swallow, especially for commands like \"git log\" that implicitly\n> use \"--default HEAD\", as they are meant for human consumption, and\n> not for scripts.\n> \n> I am not offhand 100% sure that a rev list with only negative revs\n> is totally useless, though.\n\nYeah, I'm uneasy that somebody is relying on the current behavior,\nespecially for scripting where we often do something like:\n\n git rev-list $new --not --all\n\nand an empty $new really should return an empty result (that's our usual\nconnectivity check, but I'd imagine some pre-receive hooks may end up\ndoing similar things). For rev-list I think you'd have to specify\n--default to trigger this behavior, so that helps. But it still makes me\nnervous.\n\nI'm _less_ uneasy with it for git-log, though I think people do have a\nhabit of scripting around it (because it knows some tricks that rev-list\ndoesn't).\n\nGiven that the mentioned use case was writing wrappers, could we hide\nthis behind:\n\n git config alias.l 'git log --default-on-negative ^origin/master'\n\nThat's obviously less convenient to type, but I think it would usually\nbe part of scripted calls (otherwise you'd know already that you have no\npositive refs and would just add \"..\" or HEAD or whatever).\n\n-Peff\n"}]}