{"thread":{"id":"44171","subject":"[PATCH v5] revision: new rev^-n shorthand for rev^n..rev","startedAt":"2016-09-27T08:36:21Z","lastAt":"2016-09-27T18:09:02Z","messageCount":2,"participants":["Vegard Nossum","Junio C Hamano"],"isPatch":true,"patchVersion":5,"patchTotal":null},"messages":[{"id":"302688","messageId":"20160927083249.31869-1-vegard.nossum@oracle.com","threadId":"44171","inReplyTo":null,"subject":"[PATCH v5] revision: new rev^-n shorthand for rev^n..rev","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2016-09-27T08:32:49Z","receivedAt":"2016-09-27T08:36:21Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"\"git log rev^..rev\" is commonly used to show all work done on and merged\nfrom a side branch. This patch introduces a shorthand \"rev^-\" for this\nand additionally allows \"rev^-$n\" to mean \"reachable from rev, excluding\nwhat is reachable from the nth parent of rev\". For example, for a\ntwo-parent merge, you can use rev^-2 to get the set of commits which were\nmade to the main branch while the topic branch was prepared.\n\nSigned-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n\n---\n[v2: Use ^- instead of % as suggested by Junio Hamano and use some\n common helper functions for parsing.]\n\n[v3: Use 'struct object_id' instead of 'char[20]' and add some tests as\n suggested by Matthieu Moy; fix missing '-' in Documentation/revisions.txt\n as suggested by Ramsay Jones; misc changelog + documentation fixes as\n suggested by Philip Oakley.]\n\n[v4: Documentation fixes and parsing rework suggested by Junio Hamano\n and add some more tests.]\n\n[v5: count parents before showing anything, misc testing changes, and\n changelog shortening as suggested by Junio Hamano, parent counting\n changes suggested by Jeff King.]\n---\n Documentation/revisions.txt  | 17 +++++++-\n builtin/rev-parse.c          | 54 +++++++++++++++++++------\n revision.c                   | 34 ++++++++++++++--\n t/t6101-rev-parse-parents.sh | 94 ++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 180 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/revisions.txt b/Documentation/revisions.txt\nindex 4bed5b1..ba11b9c 100644\n--- a/Documentation/revisions.txt\n+++ b/Documentation/revisions.txt\n@@ -283,7 +283,7 @@ empty range that is both reachable and unreachable from HEAD.\n \n Other <rev>{caret} Parent Shorthand Notations\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n-Two other shorthands exist, particularly useful for merge commits,\n+Three other shorthands exist, particularly useful for merge commits,\n for naming a set that is formed by a commit and its parent commits.\n \n The 'r1{caret}@' notation means all parents of 'r1'.\n@@ -291,8 +291,15 @@ The 'r1{caret}@' notation means all parents of 'r1'.\n The 'r1{caret}!' notation includes commit 'r1' but excludes all of its parents.\n By itself, this notation denotes the single commit 'r1'.\n \n+The '<rev>{caret}-{<n>}' notation includes '<rev>' but excludes the <n>th\n+parent (i.e. a shorthand for '<rev>{caret}<n>..<rev>'), with '<n>' = 1 if\n+not given. This is typically useful for merge commits where you\n+can just pass '<commit>{caret}-' to get all the commits in the branch\n+that was merged in merge commit '<commit>' (including '<commit>'\n+itself).\n+\n While '<rev>{caret}<n>' was about specifying a single commit parent, these\n-two notations consider all its parents. For example you can say\n+three notations also consider its parents. For example you can say\n 'HEAD{caret}2{caret}@', however you cannot say 'HEAD{caret}@{caret}2'.\n \n Revision Range Summary\n@@ -326,6 +333,10 @@ Revision Range Summary\n   as giving commit '<rev>' and then all its parents prefixed with\n   '{caret}' to exclude them (and their ancestors).\n \n+'<rev>{caret}-{<n>}', e.g. 'HEAD{caret}-, HEAD{caret}-2'::\n+\tEquivalent to '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n+\tgiven.\n+\n Here are a handful of examples using the Loeliger illustration above,\n with each step in the notation's expansion and selection carefully\n spelt out:\n@@ -339,6 +350,8 @@ spelt out:\n    C                            I J F C\n    B..C   = ^B C                C\n    B...C  = B ^F C              G H D E B C\n+   B^-    = B^..B\n+\t  = ^B^1 B              E I J F B\n    C^@    = C^1\n \t  = F                   I J F\n    B^@    = B^1 B^2 B^3\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 76cf05e..4da1f1d 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -298,14 +298,30 @@ static int try_parent_shorthands(const char *arg)\n \tunsigned char sha1[20];\n \tstruct commit *commit;\n \tstruct commit_list *parents;\n-\tint parents_only;\n-\n-\tif ((dotdot = strstr(arg, \"^!\")))\n-\t\tparents_only = 0;\n-\telse if ((dotdot = strstr(arg, \"^@\")))\n-\t\tparents_only = 1;\n-\n-\tif (!dotdot || dotdot[2])\n+\tint parent_number;\n+\tint include_rev = 0;\n+\tint include_parents = 0;\n+\tint exclude_parent = 0;\n+\n+\tif ((dotdot = strstr(arg, \"^!\"))) {\n+\t\tinclude_rev = 1;\n+\t\tif (dotdot[2])\n+\t\t\treturn 0;\n+\t} else if ((dotdot = strstr(arg, \"^@\"))) {\n+\t\tinclude_parents = 1;\n+\t\tif (dotdot[2])\n+\t\t\treturn 0;\n+\t} else if ((dotdot = strstr(arg, \"^-\"))) {\n+\t\tinclude_rev = 1;\n+\t\texclude_parent = 1;\n+\n+\t\tif (dotdot[2]) {\n+\t\t\tchar *end;\n+\t\t\texclude_parent = strtoul(dotdot + 2, &end, 10);\n+\t\t\tif (*end != '\\0' || !exclude_parent)\n+\t\t\t\treturn 0;\n+\t\t}\n+\t} else\n \t\treturn 0;\n \n \t*dotdot = 0;\n@@ -314,12 +330,24 @@ static int try_parent_shorthands(const char *arg)\n \t\treturn 0;\n \t}\n \n-\tif (!parents_only)\n-\t\tshow_rev(NORMAL, sha1, arg);\n \tcommit = lookup_commit_reference(sha1);\n-\tfor (parents = commit->parents; parents; parents = parents->next)\n-\t\tshow_rev(parents_only ? NORMAL : REVERSED,\n-\t\t\t\tparents->item->object.oid.hash, arg);\n+\tif (exclude_parent &&\n+\t    exclude_parent > commit_list_count(commit->parents)) {\n+\t\t*dotdot = '^';\n+\t\treturn 0;\n+\t}\n+\n+\tif (include_rev)\n+\t\tshow_rev(NORMAL, sha1, arg);\n+\tfor (parents = commit->parents, parent_number = 1;\n+\t     parents;\n+\t     parents = parents->next, parent_number++) {\n+\t\tif (exclude_parent && parent_number != exclude_parent)\n+\t\t\tcontinue;\n+\n+\t\tshow_rev(include_parents ? NORMAL : REVERSED,\n+\t\t\t parents->item->object.oid.hash, arg);\n+\t}\n \n \t*dotdot = '^';\n \treturn 1;\ndiff --git a/revision.c b/revision.c\nindex 969b3d1..b37dbec 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1289,12 +1289,14 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)\n \t}\n }\n \n-static int add_parents_only(struct rev_info *revs, const char *arg_, int flags)\n+static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,\n+\t\t\t    int exclude_parent)\n {\n \tunsigned char sha1[20];\n \tstruct object *it;\n \tstruct commit *commit;\n \tstruct commit_list *parents;\n+\tint parent_number;\n \tconst char *arg = arg_;\n \n \tif (*arg == '^') {\n@@ -1316,7 +1318,15 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags)\n \tif (it->type != OBJ_COMMIT)\n \t\treturn 0;\n \tcommit = (struct commit *)it;\n-\tfor (parents = commit->parents; parents; parents = parents->next) {\n+\tif (exclude_parent &&\n+\t    exclude_parent > commit_list_count(commit->parents))\n+\t\treturn 0;\n+\tfor (parents = commit->parents, parent_number = 1;\n+\t     parents;\n+\t     parents = parents->next, parent_number++) {\n+\t\tif (exclude_parent && parent_number != exclude_parent)\n+\t\t\tcontinue;\n+\n \t\tit = &parents->item->object;\n \t\tit->flags |= flags;\n \t\tadd_rev_cmdline(revs, it, arg_, REV_CMD_PARENTS_ONLY, flags);\n@@ -1519,17 +1529,33 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi\n \t\t}\n \t\t*dotdot = '.';\n \t}\n+\n \tdotdot = strstr(arg, \"^@\");\n \tif (dotdot && !dotdot[2]) {\n \t\t*dotdot = 0;\n-\t\tif (add_parents_only(revs, arg, flags))\n+\t\tif (add_parents_only(revs, arg, flags, 0))\n \t\t\treturn 0;\n \t\t*dotdot = '^';\n \t}\n \tdotdot = strstr(arg, \"^!\");\n \tif (dotdot && !dotdot[2]) {\n \t\t*dotdot = 0;\n-\t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM)))\n+\t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), 0))\n+\t\t\t*dotdot = '^';\n+\t}\n+\tdotdot = strstr(arg, \"^-\");\n+\tif (dotdot) {\n+\t\tint exclude_parent = 1;\n+\n+\t\tif (dotdot[2]) {\n+\t\t\tchar *end;\n+\t\t\texclude_parent = strtoul(dotdot + 2, &end, 10);\n+\t\t\tif (*end != '\\0' || !exclude_parent)\n+\t\t\t\treturn -1;\n+\t\t}\n+\n+\t\t*dotdot = 0;\n+\t\tif (!add_parents_only(revs, arg, flags ^ (UNINTERESTING | BOTTOM), exclude_parent))\n \t\t\t*dotdot = '^';\n \t}\n \ndiff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh\nindex 1c6952d..64a9850 100755\n--- a/t/t6101-rev-parse-parents.sh\n+++ b/t/t6101-rev-parse-parents.sh\n@@ -102,4 +102,98 @@ test_expect_success 'short SHA-1 works' '\n \ttest_cmp_rev_output start \"git rev-parse ${start%?}\"\n '\n \n+# rev^- tests; we can use a simpler setup for these\n+\n+test_expect_success 'setup for rev^- tests' '\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\ttest_commit three &&\n+\n+\t# Merge in a branch for testing rev^-\n+\tgit checkout -b branch &&\n+\tgit checkout HEAD^^ &&\n+\tgit merge -m merge --no-edit --no-ff branch &&\n+\tgit checkout -b merge\n+'\n+\n+# The merged branch has 2 commits + the merge\n+test_expect_success 'rev-list --count merge^- = merge^..merge' '\n+\tgit rev-list --count merge^..merge >expect &&\n+\techo 3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+# All rev^- rev-parse tests\n+\n+test_expect_success 'rev-parse merge^- = merge^..merge' '\n+\tgit rev-parse merge^..merge >expect &&\n+\tgit rev-parse merge^- >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rev-parse merge^-1 = merge^..merge' '\n+\tgit rev-parse merge^1..merge >expect &&\n+\tgit rev-parse merge^-1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rev-parse merge^-2 = merge^2..merge' '\n+\tgit rev-parse merge^2..merge >expect &&\n+\tgit rev-parse merge^-2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rev-parse merge^-0 (invalid parent)' '\n+\ttest_must_fail git rev-parse merge^-0\n+'\n+\n+test_expect_success 'rev-parse merge^-3 (invalid parent)' '\n+\ttest_must_fail git rev-parse merge^-3\n+'\n+\n+test_expect_success 'rev-parse merge^-^ (garbage after ^-)' '\n+\ttest_must_fail git rev-parse merge^-^\n+'\n+\n+test_expect_success 'rev-parse merge^-1x (garbage after ^-1)' '\n+\ttest_must_fail git rev-parse merge^-1x\n+'\n+\n+# All rev^- rev-list tests (should be mostly the same as rev-parse; the reason\n+# for the duplication is that rev-parse and rev-list use different parsers).\n+\n+test_expect_success 'rev-list merge^- = merge^..merge' '\n+\tgit rev-list merge^..merge >expect &&\n+\tgit rev-list merge^- >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rev-list merge^-1 = merge^1..merge' '\n+\tgit rev-list merge^1..merge >expect &&\n+\tgit rev-list merge^-1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rev-list merge^-2 = merge^2..merge' '\n+\tgit rev-list merge^2..merge >expect &&\n+\tgit rev-list merge^-2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rev-list merge^-0 (invalid parent)' '\n+\ttest_must_fail git rev-list merge^-0\n+'\n+\n+test_expect_success 'rev-list merge^-3 (invalid parent)' '\n+\ttest_must_fail git rev-list merge^-3\n+'\n+\n+test_expect_success 'rev-list merge^-^ (garbage after ^-)' '\n+\ttest_must_fail git rev-list merge^-^\n+'\n+\n+test_expect_success 'rev-list merge^-1x (garbage after ^-1)' '\n+\ttest_must_fail git rev-list merge^-1x\n+'\n+\n test_done\n-- \n2.10.0.rc0.1.g07c9292\n\n"},{"id":"302729","messageId":"xmqqy42dnrl8.fsf@gitster.mtv.corp.google.com","threadId":"44171","inReplyTo":"20160927083249.31869-1-vegard.nossum@oracle.com","subject":"Re: [PATCH v5] revision: new rev^-n shorthand for rev^n..rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-27T18:08:19Z","receivedAt":"2016-09-27T18:09:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vegard Nossum <vegard.nossum@oracle.com> writes:\n\n> \"git log rev^..rev\" is commonly used to show all work done on and merged\n> from a side branch. This patch introduces a shorthand \"rev^-\" for this\n> and additionally allows \"rev^-$n\" to mean \"reachable from rev, excluding\n> what is reachable from the nth parent of rev\". For example, for a\n> two-parent merge, you can use rev^-2 to get the set of commits which were\n> made to the main branch while the topic branch was prepared.\n>\n> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n\nVery nicely done.  Thanks for a pleasant read.\n\nWill queue.\n"}]}