{"thread":{"id":"44152","subject":"[RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","startedAt":"2016-09-25T08:58:15Z","lastAt":"2016-09-26T14:46:01Z","messageCount":7,"participants":["Vegard Nossum","Matthieu Moy","Ramsay Jones","Jakub Narębski","Philip Oakley","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"302509","messageId":"20160925085511.12515-1-vegard.nossum@oracle.com","threadId":"44152","inReplyTo":null,"subject":"[RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Vegard Nossum","fromEmail":"vegard.nossum@oracle.com","sentAt":"2016-09-25T08:55:11Z","receivedAt":"2016-09-25T08:58:15Z","isPatch":true,"sender":{"key":"vegard.nossum@oracle.com","avatar":"https://avatars.githubusercontent.com/u/24173?v=4"},"body":"I use rev^..rev daily, and I'm surely not the only one. To save typing\n(or copy-pasting, if the rev is long -- like a full SHA-1 or branch name)\nwe can make rev^- a shorthand for that.\n\nThe existing syntax rev^! seems like it should do the same, but it\ndoesn't really do the right thing for merge commits (it gives only the\nmerge itself).\n\nAs a natural generalisation, we also accept rev^-n where n excludes the\nnth parent of rev, although this is expected to be generally less useful.\n\n[v2: Use ^- instead of % as suggested by Junio Hamano and use some\n common helper functions for parsing.]\n\nSigned-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n---\n Documentation/revisions.txt | 14 +++++++\n builtin/rev-parse.c         | 28 ++++++++++++++\n revision.c                  | 91 +++++++++++++++++++++++++++++++++++++++++++++\n revision.h                  |  1 +\n 4 files changed, 134 insertions(+)\n\ndiff --git Documentation/revisions.txt Documentation/revisions.txt\nindex 4bed5b1..6e33801 100644\n--- Documentation/revisions.txt\n+++ Documentation/revisions.txt\n@@ -281,6 +281,14 @@ is a shorthand for 'HEAD..origin' and asks \"What did the origin do since\n I forked from them?\"  Note that '..' would mean 'HEAD..HEAD' which is an\n empty range that is both reachable and unreachable from HEAD.\n \n+Parent Exclusion Notation\n+~~~~~~~~~~~~~~~~~~~~~~~~~\n+The '<rev>{caret}-{<n>}', Parent Exclusion Notation::\n+Shorthand for '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n+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>'.\n+\n Other <rev>{caret} Parent Shorthand Notations\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n Two other shorthands exist, particularly useful for merge commits,\n@@ -316,6 +324,10 @@ Revision Range Summary\n \t<rev2> but exclude those that are reachable from both.  When\n \teither <rev1> or <rev2> is omitted, it defaults to `HEAD`.\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 '<rev>{caret}@', e.g. 'HEAD{caret}@'::\n   A suffix '{caret}' followed by an at sign is the same as listing\n   all parents of '<rev>' (meaning, include anything reachable from\n@@ -339,6 +351,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 ^B^1              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 builtin/rev-parse.c builtin/rev-parse.c\nindex 76cf05e..ad5e6ac 100644\n--- builtin/rev-parse.c\n+++ builtin/rev-parse.c\n@@ -292,6 +292,32 @@ static int try_difference(const char *arg)\n \treturn 0;\n }\n \n+static int try_parent_exclusion(const char *arg)\n+{\n+\tint ret = 0;\n+\tchar *to_rev = NULL;\n+\tchar *from_rev = NULL;\n+\tunsigned char to_sha1[20];\n+\tunsigned char from_sha1[20];\n+\n+\tif (parse_parent_exclusion(arg, &to_rev, &from_rev))\n+\t\tgoto out;\n+\tif (get_sha1_committish(to_rev, to_sha1))\n+\t\tgoto out;\n+\tif (get_sha1_committish(from_rev, from_sha1))\n+\t\tgoto out;\n+\n+\tshow_rev(NORMAL, to_sha1, to_rev);\n+\tshow_rev(REVERSED, from_sha1, from_rev);\n+\n+\tret = 1;\n+\n+out:\n+\tfree(to_rev);\n+\tfree(from_rev);\n+\treturn ret;\n+}\n+\n static int try_parent_shorthands(const char *arg)\n {\n \tchar *dotdot;\n@@ -839,6 +865,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t/* Not a flag argument */\n \t\tif (try_difference(arg))\n \t\t\tcontinue;\n+\t\tif (try_parent_exclusion(arg))\n+\t\t\tcontinue;\n \t\tif (try_parent_shorthands(arg))\n \t\t\tcontinue;\n \t\tname = arg;\ndiff --git revision.c revision.c\nindex 969b3d1..0480f19 100644\n--- revision.c\n+++ revision.c\n@@ -1419,6 +1419,93 @@ static void prepare_show_merge(struct rev_info *revs)\n \trevs->limited = 1;\n }\n \n+/*\n+ * If 'arg' is on the form '<rev>^-{<n>}', then return 0 and\n+ * '*to_rev' and '*from_rev' will contain '<rev>' and '<rev>^<n>',\n+ * respectively.\n+ */\n+int parse_parent_exclusion(const char *arg, char **to_rev, char **from_rev)\n+{\n+\tchar *caret;\n+\tunsigned int n = 1;\n+\n+\t/*\n+\t * <rev>^-{<n>} is shorthand for <rev>^<n>..<rev>, with <n> = 1 if\n+\t * not given. This is typically used for merge commits where you\n+\t * can just pass '<merge>^-' and it will show you all the commits in\n+\t * the branch that was merged.\n+\t */\n+\n+\tif (!(caret = strstr(arg, \"^-\")))\n+\t\treturn 1;\n+\tif (caret[2]) {\n+\t\tchar *end;\n+\t\tn = strtoul(&caret[2], &end, 10);\n+\t\tif (*end != '\\0')\n+\t\t\treturn 1;\n+\t}\n+\t*to_rev = xstrndup(arg, caret - arg);\n+\t*from_rev = xstrfmt(\"%s^%u\", *to_rev, n);\n+\treturn 0;\n+}\n+\n+static int handle_parent_exclusion(const char *arg, struct rev_info *revs, int flags)\n+{\n+\tint ret = 1;\n+\tchar *to_rev = NULL;\n+\tchar *from_rev = NULL;\n+\tunsigned char to_sha1[20];\n+\tunsigned char from_sha1[20];\n+\n+\tstruct object *a_obj, *b_obj;\n+\tunsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);\n+\tunsigned int a_flags;\n+\n+\t/*\n+\t * <rev>^-{<n>} is shorthand for <rev>^<n>..<rev>, with <n> = 1 if\n+\t * not given. This is typically used for merge commits where you\n+\t * can just pass <merge>^- and it will show you all the commits in\n+\t * the branches that were merged.\n+\t */\n+\n+\tif (parse_parent_exclusion(arg, &to_rev, &from_rev))\n+\t\tgoto out;\n+\n+\tif (get_sha1_committish(to_rev, to_sha1)) {\n+\t\tif (revs->ignore_missing)\n+\t\t\tgoto out;\n+\t\tdie(\"Unknown revision %s\", to_rev);\n+\t}\n+\n+\tif (get_sha1_committish(from_rev, from_sha1)) {\n+\t\tif (revs->ignore_missing)\n+\t\t\tgoto out;\n+\t\tdie(\"Unknown revision %s\", from_rev);\n+\t}\n+\n+\ta_obj = parse_object(from_sha1);\n+\tb_obj = parse_object(to_sha1);\n+\tif (!a_obj || !b_obj) {\n+\t\tif (revs->ignore_missing)\n+\t\t\tgoto out;\n+\t\tdie(\"Invalid revision range %s\", arg);\n+\t}\n+\n+\ta_flags = flags_exclude;\n+\ta_obj->flags |= a_flags;\n+\tb_obj->flags |= flags;\n+\tadd_rev_cmdline(revs, a_obj, from_rev, REV_CMD_LEFT, a_flags);\n+\tadd_pending_object(revs, a_obj, from_rev);\n+\tadd_rev_cmdline(revs, b_obj, to_rev, REV_CMD_RIGHT, flags);\n+\tadd_pending_object(revs, b_obj, to_rev);\n+\n+\tret = 0;\n+out:\n+\tfree(to_rev);\n+\tfree(from_rev);\n+\treturn ret;\n+}\n+\n int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt)\n {\n \tstruct object_context oc;\n@@ -1519,6 +1606,10 @@ 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+\tif (!handle_parent_exclusion(arg, revs, flags))\n+\t\treturn 0;\n+\n \tdotdot = strstr(arg, \"^@\");\n \tif (dotdot && !dotdot[2]) {\n \t\t*dotdot = 0;\ndiff --git revision.h revision.h\nindex 9fac1a6..ca5bebc 100644\n--- revision.h\n+++ revision.h\n@@ -243,6 +243,7 @@ extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,\n extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \t\t\t       const struct option *options,\n \t\t\t       const char * const usagestr[]);\n+extern int parse_parent_exclusion(const char *arg, char **to_rev, char **from_rev);\n #define REVARG_CANNOT_BE_FILENAME 01\n #define REVARG_COMMITTISH 02\n extern int handle_revision_arg(const char *arg, struct rev_info *revs,\n-- \n2.10.0.rc0.1.g07c9292\n\n"},{"id":"302511","messageId":"vpq4m54wa1l.fsf@anie.imag.fr","threadId":"44152","inReplyTo":"20160925085511.12515-1-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-09-25T10:25:42Z","receivedAt":"2016-09-25T10:26:18Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Vegard Nossum <vegard.nossum@oracle.com> writes:\n\n>  Documentation/revisions.txt | 14 +++++++\n>  builtin/rev-parse.c         | 28 ++++++++++++++\n>  revision.c                  | 91 +++++++++++++++++++++++++++++++++++++++++++++\n>  revision.h                  |  1 +\n\nThis would obviously need tests before you can drop the RFC flag.\n\n> --- builtin/rev-parse.c\n> +++ builtin/rev-parse.c\n> @@ -292,6 +292,32 @@ static int try_difference(const char *arg)\n>  \treturn 0;\n>  }\n>  \n> +static int try_parent_exclusion(const char *arg)\n> +{\n> +\tint ret = 0;\n> +\tchar *to_rev = NULL;\n> +\tchar *from_rev = NULL;\n> +\tunsigned char to_sha1[20];\n> +\tunsigned char from_sha1[20];\n\nI didn't follow closely, but there are patch series by brian m. carlson\nto convert these unsigned char array[20] to struct object_id. I guess\nadding more arrays is going in the wrong direction. You may want to Cc\nbrian if unsure.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"302517","messageId":"b4fda9ca-7dab-dee7-d963-0555a372b345@ramsayjones.plus.com","threadId":"44152","inReplyTo":"20160925085511.12515-1-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-09-25T14:07:24Z","receivedAt":"2016-09-25T14:07:35Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 25/09/16 09:55, Vegard Nossum wrote:\n> I use rev^..rev daily, and I'm surely not the only one. To save typing\n> (or copy-pasting, if the rev is long -- like a full SHA-1 or branch name)\n> we can make rev^- a shorthand for that.\n> \n> The existing syntax rev^! seems like it should do the same, but it\n> doesn't really do the right thing for merge commits (it gives only the\n> merge itself).\n> \n> As a natural generalisation, we also accept rev^-n where n excludes the\n> nth parent of rev, although this is expected to be generally less useful.\n> \n> [v2: Use ^- instead of % as suggested by Junio Hamano and use some\n>  common helper functions for parsing.]\n\nI would place this v2 commentary below the '---' marker (so that it\nwon't appear in the commit message) ...\n\n> \n> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n> ---\n\n... here.\n\n>  Documentation/revisions.txt | 14 +++++++\n>  builtin/rev-parse.c         | 28 ++++++++++++++\n>  revision.c                  | 91 +++++++++++++++++++++++++++++++++++++++++++++\n>  revision.h                  |  1 +\n>  4 files changed, 134 insertions(+)\n> \n> diff --git Documentation/revisions.txt Documentation/revisions.txt\n> index 4bed5b1..6e33801 100644\n> --- Documentation/revisions.txt\n> +++ Documentation/revisions.txt\n> @@ -281,6 +281,14 @@ is a shorthand for 'HEAD..origin' and asks \"What did the origin do since\n>  I forked from them?\"  Note that '..' would mean 'HEAD..HEAD' which is an\n>  empty range that is both reachable and unreachable from HEAD.\n>  \n> +Parent Exclusion Notation\n> +~~~~~~~~~~~~~~~~~~~~~~~~~\n> +The '<rev>{caret}-{<n>}', Parent Exclusion Notation::\n> +Shorthand for '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n> +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>'.\n> +\n>  Other <rev>{caret} Parent Shorthand Notations\n>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>  Two other shorthands exist, particularly useful for merge commits,\n> @@ -316,6 +324,10 @@ Revision Range Summary\n>  \t<rev2> but exclude those that are reachable from both.  When\n>  \teither <rev1> or <rev2> is omitted, it defaults to `HEAD`.\n>  \n> +'<rev>{caret}-{<n>}', e.g. 'HEAD{caret}, HEAD{caret}-2'::\n\nmissing '-' ------------------------------^\n\n> +\tEquivalent to '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n> +\tgiven.\n> +\n>  '<rev>{caret}@', e.g. 'HEAD{caret}@'::\n>    A suffix '{caret}' followed by an at sign is the same as listing\n>    all parents of '<rev>' (meaning, include anything reachable from\n> @@ -339,6 +351,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 ^B^1              E I J F B\n>     C^@    = C^1\n>  \t  = F                   I J F\n>     B^@    = B^1 B^2 B^3\n> diff --git builtin/rev-parse.c builtin/rev-parse.c\n> index 76cf05e..ad5e6ac 100644\n> --- builtin/rev-parse.c\n> +++ builtin/rev-parse.c\n> @@ -292,6 +292,32 @@ static int try_difference(const char *arg)\n>  \treturn 0;\n>  }\n>  \n> +static int try_parent_exclusion(const char *arg)\n> +{\n> +\tint ret = 0;\n> +\tchar *to_rev = NULL;\n> +\tchar *from_rev = NULL;\n> +\tunsigned char to_sha1[20];\n> +\tunsigned char from_sha1[20];\n\nAs Matthieu already mentioned, maybe use 'struct object_id' here.\n\n> +\n> +\tif (parse_parent_exclusion(arg, &to_rev, &from_rev))\n> +\t\tgoto out;\n> +\tif (get_sha1_committish(to_rev, to_sha1))\n\n... then 'to_sha1.hash' here, etc ...\n\nATB,\nRamsay Jones\n\n"},{"id":"302519","messageId":"ab725c6e-5c8f-d137-fefc-7f094ea7b7bb@gmail.com","threadId":"44152","inReplyTo":"20160925085511.12515-1-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-25T14:19:58Z","receivedAt":"2016-09-25T14:20:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 25.09.2016 o 10:55, Vegard Nossum pisze:\n> I use rev^..rev daily, and I'm surely not the only one. To save typing\n> (or copy-pasting, if the rev is long -- like a full SHA-1 or branch name)\n> we can make rev^- a shorthand for that.\n> \n> The existing syntax rev^! seems like it should do the same, but it\n> doesn't really do the right thing for merge commits (it gives only the\n> merge itself).\n> \n> As a natural generalisation, we also accept rev^-n where n excludes the\n> nth parent of rev, although this is expected to be generally less useful.\n> \n> [v2: Use ^- instead of % as suggested by Junio Hamano and use some\n>  common helper functions for parsing.]\n\nMinor sidenote: the above should go after the \"---\" line, as it should\nbe not included in the final commit message.\n\n> \n> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n> ---\n>  Documentation/revisions.txt | 14 +++++++\n>  builtin/rev-parse.c         | 28 ++++++++++++++\n>  revision.c                  | 91 +++++++++++++++++++++++++++++++++++++++++++++\n>  revision.h                  |  1 +\n>  4 files changed, 134 insertions(+)\n\n"},{"id":"302525","messageId":"E7D489891E404823BF34F48E7B9E5618@PhilipOakley","threadId":"44152","inReplyTo":"20160925085511.12515-1-vegard.nossum@oracle.com","subject":"Re: [RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-25T17:37:42Z","receivedAt":"2016-09-25T17:45:59Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Vegard Nossum\" <vegard.nossum@oracle.com>\n>I use rev^..rev daily, and I'm surely not the only one.\n\nNot everyone knows the 'trick' and may not use it daily.\n\nConsider stating what it is useful for (e.g. \"useful to get the commits and \nall  commits in the branches that were merged into commit\" - paraphrased \nfrom the doc text)\n\n> To save typing\n> (or copy-pasting, if the rev is long -- like a full SHA-1 or branch name)\n> we can make rev^- a shorthand for that.\n>\n> The existing syntax rev^! seems like it should do the same, but it\n> doesn't really do the right thing for merge commits (it gives only the\n> merge itself).\n\n.. rather than the commit and those on side branches).\n\n>\n> As a natural generalisation, we also accept rev^-n where n excludes the\n> nth parent of rev,\n\n> although this is expected to be generally less useful.\n\nPresumptious? for a two parent merge, surely(?) rev^-2 will give you what \nhas been going on on the main line while the branch was being prepared... \ncompare A^- and A^-2.\n\n>\n> [v2: Use ^- instead of % as suggested by Junio Hamano and use some\n> common helper functions for parsing.]\n\nAs others noted, stick the note below a three dash line following the sign \noff (it can be part of the commit message after the sign off. It's also a \nuseful place for including any cc: list when using format-patch and \nsend-email.\n>\n> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>\n> ---\n> Documentation/revisions.txt | 14 +++++++\n> builtin/rev-parse.c         | 28 ++++++++++++++\n> revision.c                  | 91 \n> +++++++++++++++++++++++++++++++++++++++++++++\n> revision.h                  |  1 +\n> 4 files changed, 134 insertions(+)\n>\n> diff --git Documentation/revisions.txt Documentation/revisions.txt\n> index 4bed5b1..6e33801 100644\n> --- Documentation/revisions.txt\n> +++ Documentation/revisions.txt\n> @@ -281,6 +281,14 @@ is a shorthand for 'HEAD..origin' and asks \"What did \n> the origin do since\n> I forked from them?\"  Note that '..' would mean 'HEAD..HEAD' which is an\n> empty range that is both reachable and unreachable from HEAD.\n>\n> +Parent Exclusion Notation\n> +~~~~~~~~~~~~~~~~~~~~~~~~~\n> +The '<rev>{caret}-{<n>}', Parent Exclusion Notation::\n> +Shorthand for '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n> +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\ns/get all the/get the commit and all the/ ?\nIt could be misread as a way of selecting just those commits that are within \nthe side branch without including the given commit itself.\n\n> +that was merged in merge commit '<commit>'.\n> +\n> Other <rev>{caret} Parent Shorthand Notations\n> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> Two other shorthands exist, particularly useful for merge commits,\n> @@ -316,6 +324,10 @@ Revision Range Summary\n>  <rev2> but exclude those that are reachable from both.  When\n>  either <rev1> or <rev2> is omitted, it defaults to `HEAD`.\n>\n> +'<rev>{caret}-{<n>}', e.g. 'HEAD{caret}, HEAD{caret}-2'::\n> + Equivalent to '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n> + given.\n> +\n> '<rev>{caret}@', e.g. 'HEAD{caret}@'::\n>   A suffix '{caret}' followed by an at sign is the same as listing\n>   all parents of '<rev>' (meaning, include anything reachable from\n> @@ -339,6 +351,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> +   = B ^B^1              E I J F B\n>    C^@    = C^1\n>    = F                   I J F\n>    B^@    = B^1 B^2 B^3\n> diff --git builtin/rev-parse.c builtin/rev-parse.c\n> index 76cf05e..ad5e6ac 100644\n> --- builtin/rev-parse.c\n> +++ builtin/rev-parse.c\n> @@ -292,6 +292,32 @@ static int try_difference(const char *arg)\n>  return 0;\n> }\n>\n> +static int try_parent_exclusion(const char *arg)\n> +{\n> + int ret = 0;\n> + char *to_rev = NULL;\n> + char *from_rev = NULL;\n> + unsigned char to_sha1[20];\n> + unsigned char from_sha1[20];\n> +\n> + if (parse_parent_exclusion(arg, &to_rev, &from_rev))\n> + goto out;\n> + if (get_sha1_committish(to_rev, to_sha1))\n> + goto out;\n> + if (get_sha1_committish(from_rev, from_sha1))\n> + goto out;\n> +\n> + show_rev(NORMAL, to_sha1, to_rev);\n> + show_rev(REVERSED, from_sha1, from_rev);\n> +\n> + ret = 1;\n> +\n> +out:\n> + free(to_rev);\n> + free(from_rev);\n> + return ret;\n> +}\n> +\n> static int try_parent_shorthands(const char *arg)\n> {\n>  char *dotdot;\n> @@ -839,6 +865,8 @@ int cmd_rev_parse(int argc, const char **argv, const \n> char *prefix)\n>  /* Not a flag argument */\n>  if (try_difference(arg))\n>  continue;\n> + if (try_parent_exclusion(arg))\n> + continue;\n>  if (try_parent_shorthands(arg))\n>  continue;\n>  name = arg;\n> diff --git revision.c revision.c\n> index 969b3d1..0480f19 100644\n> --- revision.c\n> +++ revision.c\n> @@ -1419,6 +1419,93 @@ static void prepare_show_merge(struct rev_info \n> *revs)\n>  revs->limited = 1;\n> }\n>\n> +/*\n> + * If 'arg' is on the form '<rev>^-{<n>}', then return 0 and\n> + * '*to_rev' and '*from_rev' will contain '<rev>' and '<rev>^<n>',\n> + * respectively.\n> + */\n> +int parse_parent_exclusion(const char *arg, char **to_rev, char \n> **from_rev)\n> +{\n> + char *caret;\n> + unsigned int n = 1;\n> +\n> + /*\n> + * <rev>^-{<n>} is shorthand for <rev>^<n>..<rev>, with <n> = 1 if\n> + * not given. This is typically used for merge commits where you\n> + * can just pass '<merge>^-' and it will show you all the commits in\n> + * the branch that was merged.\n> + */\n> +\n> + if (!(caret = strstr(arg, \"^-\")))\n> + return 1;\n> + if (caret[2]) {\n> + char *end;\n> + n = strtoul(&caret[2], &end, 10);\n> + if (*end != '\\0')\n> + return 1;\n> + }\n> + *to_rev = xstrndup(arg, caret - arg);\n> + *from_rev = xstrfmt(\"%s^%u\", *to_rev, n);\n> + return 0;\n> +}\n> +\n> +static int handle_parent_exclusion(const char *arg, struct rev_info \n> *revs, int flags)\n> +{\n> + int ret = 1;\n> + char *to_rev = NULL;\n> + char *from_rev = NULL;\n> + unsigned char to_sha1[20];\n> + unsigned char from_sha1[20];\n> +\n> + struct object *a_obj, *b_obj;\n> + unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);\n> + unsigned int a_flags;\n> +\n> + /*\n> + * <rev>^-{<n>} is shorthand for <rev>^<n>..<rev>, with <n> = 1 if\n> + * not given. This is typically used for merge commits where you\n> + * can just pass <merge>^- and it will show you all the commits in\n> + * the branches that were merged.\n> + */\n> +\n> + if (parse_parent_exclusion(arg, &to_rev, &from_rev))\n> + goto out;\n> +\n> + if (get_sha1_committish(to_rev, to_sha1)) {\n> + if (revs->ignore_missing)\n> + goto out;\n> + die(\"Unknown revision %s\", to_rev);\n> + }\n> +\n> + if (get_sha1_committish(from_rev, from_sha1)) {\n> + if (revs->ignore_missing)\n> + goto out;\n> + die(\"Unknown revision %s\", from_rev);\n> + }\n> +\n> + a_obj = parse_object(from_sha1);\n> + b_obj = parse_object(to_sha1);\n> + if (!a_obj || !b_obj) {\n> + if (revs->ignore_missing)\n> + goto out;\n> + die(\"Invalid revision range %s\", arg);\n> + }\n> +\n> + a_flags = flags_exclude;\n> + a_obj->flags |= a_flags;\n> + b_obj->flags |= flags;\n> + add_rev_cmdline(revs, a_obj, from_rev, REV_CMD_LEFT, a_flags);\n> + add_pending_object(revs, a_obj, from_rev);\n> + add_rev_cmdline(revs, b_obj, to_rev, REV_CMD_RIGHT, flags);\n> + add_pending_object(revs, b_obj, to_rev);\n> +\n> + ret = 0;\n> +out:\n> + free(to_rev);\n> + free(from_rev);\n> + return ret;\n> +}\n> +\n> int handle_revision_arg(const char *arg_, struct rev_info *revs, int \n> flags, unsigned revarg_opt)\n> {\n>  struct object_context oc;\n> @@ -1519,6 +1606,10 @@ int handle_revision_arg(const char *arg_, struct \n> rev_info *revs, int flags, unsi\n>  }\n>  *dotdot = '.';\n>  }\n> +\n> + if (!handle_parent_exclusion(arg, revs, flags))\n> + return 0;\n> +\n>  dotdot = strstr(arg, \"^@\");\n>  if (dotdot && !dotdot[2]) {\n>  *dotdot = 0;\n> diff --git revision.h revision.h\n> index 9fac1a6..ca5bebc 100644\n> --- revision.h\n> +++ revision.h\n> @@ -243,6 +243,7 @@ extern int setup_revisions(int argc, const char \n> **argv, struct rev_info *revs,\n> extern void parse_revision_opt(struct rev_info *revs, struct \n> parse_opt_ctx_t *ctx,\n>         const struct option *options,\n>         const char * const usagestr[]);\n> +extern int parse_parent_exclusion(const char *arg, char **to_rev, char \n> **from_rev);\n> #define REVARG_CANNOT_BE_FILENAME 01\n> #define REVARG_COMMITTISH 02\n> extern int handle_revision_arg(const char *arg, struct rev_info *revs,\n> -- \n> 2.10.0.rc0.1.g07c9292\n>\n> \n\n"},{"id":"302548","messageId":"xmqq7f9zwl2q.fsf@gitster.mtv.corp.google.com","threadId":"44152","inReplyTo":"E7D489891E404823BF34F48E7B9E5618@PhilipOakley","subject":"Re: [RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-26T00:39:41Z","receivedAt":"2016-09-26T00:39:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" <philipoakley@iee.org> writes:\n\n> From: \"Vegard Nossum\" <vegard.nossum@oracle.com>\n>>I use rev^..rev daily, and I'm surely not the only one.\n>\n> Not everyone knows the 'trick' and may not use it daily.\n>\n> Consider stating what it is useful for (e.g. \"useful to get the\n> commits and all  commits in the branches that were merged into commit\"\n> - paraphrased from the doc text)\n>\n>> To save typing\n>> (or copy-pasting, if the rev is long -- like a full SHA-1 or branch name)\n>> we can make rev^- a shorthand for that.\n>>\n>> The existing syntax rev^! seems like it should do the same, but it\n>> doesn't really do the right thing for merge commits (it gives only the\n>> merge itself).\n>\n> .. rather than the commit and those on side branches).\n>> As a natural generalisation, we also accept rev^-n where n excludes the\n>> nth parent of rev,\n>\n>> although this is expected to be generally less useful.\n>\n> Presumptious? for a two parent merge, surely(?) rev^-2 will give you\n> what has been going on on the main line while the branch was being\n> prepared... compare A^- and A^-2.\n\nAll good comments.  It often is a good strategy to avoid subjective\n\"this is useful\" and \"this is not useful\" assessment, and instead\nlet the feature itself find its supporters in the reading public.\n\n>> +Parent Exclusion Notation\n>> +~~~~~~~~~~~~~~~~~~~~~~~~~\n>> +The '<rev>{caret}-{<n>}', Parent Exclusion Notation::\n>> +Shorthand for '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n>> +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>\n> s/get all the/get the commit and all the/ ?\n> It could be misread as a way of selecting just those commits that are\n> within the side branch without including the given commit itself.\n>\n>> +that was merged in merge commit '<commit>'.\n>> +\n>> Other <rev>{caret} Parent Shorthand Notations\n>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>> Two other shorthands exist, particularly useful for merge commits,\n\nIs it just me that this new thing belongs to this \"other shorthand\nnotations\", making the total to three from two?  It really is a\nclosely related cousin of existing 'r1{caret}!'; instead of\nexcluding all of its parents, it only excludes the specified one of\nits parents.  IOW, this new one is better described as the third\nother shorthand in this \"Other Notations\" section, without creating\na new \"Parent Exclusion Notation\" section.\n\n>> @@ -316,6 +324,10 @@ Revision Range Summary\n>>  <rev2> but exclude those that are reachable from both.  When\n>>  either <rev1> or <rev2> is omitted, it defaults to `HEAD`.\n>>\n>> +'<rev>{caret}-{<n>}', e.g. 'HEAD{caret}, HEAD{caret}-2'::\n\nHuh?  Isn't the first example missing the necessary minus sign?\n\n>> + Equivalent to '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n>> + given.\n>> +\n>> '<rev>{caret}@', e.g. 'HEAD{caret}@'::\n>>   A suffix '{caret}' followed by an at sign is the same as listing\n>>   all parents of '<rev>' (meaning, include anything reachable from\n>> @@ -339,6 +351,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>> +   = B ^B^1              E I J F B\n\nEven though these are order independent, the second line should say\n\n   = ^B^1 B              E I J F B\n\nto be consistent with the expansion of B..C, I would think.  \n\n>> diff --git builtin/rev-parse.c builtin/rev-parse.c\n>> index 76cf05e..ad5e6ac 100644\n>> --- builtin/rev-parse.c\n>> +++ builtin/rev-parse.c\n>> @@ -292,6 +292,32 @@ static int try_difference(const char *arg)\n>>  return 0;\n>> }\n>>\n>> +static int try_parent_exclusion(const char *arg)\n>> +{\n>> + int ret = 0;\n>> + char *to_rev = NULL;\n>> + char *from_rev = NULL;\n>> + unsigned char to_sha1[20];\n>> + unsigned char from_sha1[20];\n>> +\n>> + if (parse_parent_exclusion(arg, &to_rev, &from_rev))\n>> + goto out;\n>> + if (get_sha1_committish(to_rev, to_sha1))\n>> + goto out;\n>> + if (get_sha1_committish(from_rev, from_sha1))\n>> + goto out;\n>> +\n>> + show_rev(NORMAL, to_sha1, to_rev);\n>> + show_rev(REVERSED, from_sha1, from_rev);\n>> +\n>> + ret = 1;\n>> +\n>> +out:\n>> + free(to_rev);\n>> + free(from_rev);\n>> + return ret;\n>> +}\n>> +\n>> static int try_parent_shorthands(const char *arg)\n>> {\n>>  char *dotdot;\n\nI did not expect that this needs an entirely new helper function,\ninstead of being implemented as a new special case of existing\ntry_parent_shorthands() function.  You'd need to strstr \"^-\" and\nparse a sequence of digits that follow it, which may want a helper\nto make sure you can error out if fed \"some^-12thing\" saying that\n\"12thing\" is not an integer, extend the existing \"parents-only\"\nthing so that it can represent three cases (i.e. @? !? or -?), and\nneed a new variable to denote which parent is to be excluded when it\nis the '-' kind.  You'd need to temporarily *dotdot = '\\0', parse\nwhat is before \"^-\" and revert *dotdot = '^' like existing helper\nfunction just the same.\n\nExactly the same comment probably applies to the changes to the\nparser in revision.c, I would imagine, but I didn't read it ;-)\n"},{"id":"302578","messageId":"1173701ACA474229ABC1A14468682474@PhilipOakley","threadId":"44152","inReplyTo":"xmqq7f9zwl2q.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC PATCH v2] revision: new rev^-n shorthand for rev^n..rev","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-26T13:00:04Z","receivedAt":"2016-09-26T14:46:01Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> \"Philip Oakley\" <philipoakley@iee.org> writes:\n>\n>> From: \"Vegard Nossum\" <vegard.nossum@oracle.com>\n>>>I use rev^..rev daily, and I'm surely not the only one.\n>>\n>> Not everyone knows the 'trick' and may not use it daily.\n>>\n>> Consider stating what it is useful for (e.g. \"useful to get the\n>> commits and all  commits in the branches that were merged into commit\"\n>> - paraphrased from the doc text)\n>>\n>>> To save typing\n>>> (or copy-pasting, if the rev is long -- like a full SHA-1 or branch \n>>> name)\n>>> we can make rev^- a shorthand for that.\n>>>\n>>> The existing syntax rev^! seems like it should do the same, but it\n>>> doesn't really do the right thing for merge commits (it gives only the\n>>> merge itself).\n>>\n>> .. rather than the commit and those on side branches).\n>>> As a natural generalisation, we also accept rev^-n where n excludes the\n>>> nth parent of rev,\n>>\n>>> although this is expected to be generally less useful.\n>>\n>> Presumptious? for a two parent merge, surely(?) rev^-2 will give you\n>> what has been going on on the main line while the branch was being\n>> prepared... compare A^- and A^-2.\n>\n> All good comments.  It often is a good strategy to avoid subjective\n> \"this is useful\" and \"this is not useful\" assessment, and instead\n> let the feature itself find its supporters in the reading public.\n>\n>>> +Parent Exclusion Notation\n>>> +~~~~~~~~~~~~~~~~~~~~~~~~~\n>>> +The '<rev>{caret}-{<n>}', Parent Exclusion Notation::\n>>> +Shorthand for '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n>>> +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>>\n>> s/get all the/get the commit and all the/ ?\n>> It could be misread as a way of selecting just those commits that are\n>> within the side branch without including the given commit itself.\n>>\n>>> +that was merged in merge commit '<commit>'.\n>>> +\n>>> Other <rev>{caret} Parent Shorthand Notations\n>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>>> Two other shorthands exist, particularly useful for merge commits,\n>\n> Is it just me that this new thing belongs to this \"other shorthand\n> notations\", making the total to three from two?  It really is a\n> closely related cousin of existing 'r1{caret}!'; instead of\n> excluding all of its parents, it only excludes the specified one of\n> its parents.  IOW, this new one is better described as the third\n> other shorthand in this \"Other Notations\" section, without creating\n> a new \"Parent Exclusion Notation\" section.\n>\nTrue. It probably should be there.\n\n>>> @@ -316,6 +324,10 @@ Revision Range Summary\n>>>  <rev2> but exclude those that are reachable from both.  When\n>>>  either <rev1> or <rev2> is omitted, it defaults to `HEAD`.\n>>>\n>>> +'<rev>{caret}-{<n>}', e.g. 'HEAD{caret}, HEAD{caret}-2'::\n>\n> Huh?  Isn't the first example missing the necessary minus sign?\n>\n>>> + Equivalent to '<rev>{caret}<n>..<rev>', with '<n>' = 1 if not\n>>> + given.\n>>> +\n>>> '<rev>{caret}@', e.g. 'HEAD{caret}@'::\n>>>   A suffix '{caret}' followed by an at sign is the same as listing\n>>>   all parents of '<rev>' (meaning, include anything reachable from\n>>> @@ -339,6 +351,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>>> +   = B ^B^1              E I J F B\n>\n> Even though these are order independent, the second line should say\n>\n>   = ^B^1 B              E I J F B\n>\n> to be consistent with the expansion of B..C, I would think.\n\nAgreed.\n>\n>>> diff --git builtin/rev-parse.c builtin/rev-parse.c\n>>> index 76cf05e..ad5e6ac 100644\n>>> --- builtin/rev-parse.c\n>>> +++ builtin/rev-parse.c\n>>> @@ -292,6 +292,32 @@ static int try_difference(const char *arg)\n>>>  return 0;\n>>> }\n>>>\n>>> +static int try_parent_exclusion(const char *arg)\n>>> +{\n>>> + int ret = 0;\n>>> + char *to_rev = NULL;\n>>> + char *from_rev = NULL;\n>>> + unsigned char to_sha1[20];\n>>> + unsigned char from_sha1[20];\n>>> +\n>>> + if (parse_parent_exclusion(arg, &to_rev, &from_rev))\n>>> + goto out;\n>>> + if (get_sha1_committish(to_rev, to_sha1))\n>>> + goto out;\n>>> + if (get_sha1_committish(from_rev, from_sha1))\n>>> + goto out;\n>>> +\n>>> + show_rev(NORMAL, to_sha1, to_rev);\n>>> + show_rev(REVERSED, from_sha1, from_rev);\n>>> +\n>>> + ret = 1;\n>>> +\n>>> +out:\n>>> + free(to_rev);\n>>> + free(from_rev);\n>>> + return ret;\n>>> +}\n>>> +\n>>> static int try_parent_shorthands(const char *arg)\n>>> {\n>>>  char *dotdot;\n>\n> I did not expect that this needs an entirely new helper function,\n> instead of being implemented as a new special case of existing\n> try_parent_shorthands() function.  You'd need to strstr \"^-\" and\n> parse a sequence of digits that follow it, which may want a helper\n> to make sure you can error out if fed \"some^-12thing\" saying that\n> \"12thing\" is not an integer, extend the existing \"parents-only\"\n> thing so that it can represent three cases (i.e. @? !? or -?), and\n> need a new variable to denote which parent is to be excluded when it\n> is the '-' kind.  You'd need to temporarily *dotdot = '\\0', parse\n> what is before \"^-\" and revert *dotdot = '^' like existing helper\n> function just the same.\n>\n> Exactly the same comment probably applies to the changes to the\n> parser in revision.c, I would imagine, but I didn't read it ;-)\n\nSounds sensible. I hadn't double checked Vegard's implementation at this \npoint.\n--\nPhilip\n> \n\n"}]}