{"thread":{"id":"39518","subject":"[PATCH 0/2] specify commit by negative pattern","startedAt":"2015-06-03T20:54:12Z","lastAt":"2015-06-04T17:09:21Z","messageCount":7,"participants":["Will Palmer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"262916","messageId":"1433364854-30088-1-git-send-email-wmpalmer@gmail.com","threadId":"39518","inReplyTo":null,"subject":"[PATCH 0/2] specify commit by negative pattern","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2015-06-03T20:54:12Z","receivedAt":"2015-06-03T20:54:12Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"add support for negative pattern matching in @^{/<pattern>} style\nrevision specifiers. So now you can find the first commit whose message\ndoesn't match a pattern, complementing the existing positive matching.\ne.g.:\n\n    $ git rebase -i @^{/!^WIP}\n\nMy use-case is in having a \"work, work, work, rebase, push\"-style\nworkflow, which generates a lot of \"WIP foo\" commits. While rebasing is\nusually handled via \"git rebase -i origin/master\", occasionally I will\nalready have several \"good, but not yet ready to push\" commits hanging\naround while I finish work on related commits. In these situations, the\nability to quickly \"git diff @^{/!^WIP}\" to get an overview of all\nchanges \"since the last one I was happy with\", can be useful.\n\nReading through the history of this type of revision specifier, it feels\nlike a negative match was always thought of as potentially useful\nsomeday, but didn't fit well with the original patch's limitations\n(namely: always searching across all refs).\n\nWill Palmer (2):\n  test for '!' handling in rev-parse's named commits\n  object name: introduce '^{/!<negative pattern>}' notation\n\n Documentation/revisions.txt |  7 ++++---\n sha1_name.c                 | 22 ++++++++++++++++------\n t/t1511-rev-parse-caret.sh  | 45 ++++++++++++++++++++++++++++++++++++++++++++-\n 3 files changed, 64 insertions(+), 10 deletions(-)\n\n-- \n2.3.0.rc1\n"},{"id":"262917","messageId":"1433364854-30088-2-git-send-email-wmpalmer@gmail.com","threadId":"39518","inReplyTo":"1433364854-30088-1-git-send-email-wmpalmer@gmail.com","subject":"[PATCH 1/2] test for '!' handling in rev-parse's named commits","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2015-06-03T20:54:13Z","receivedAt":"2015-06-03T20:54:13Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"In anticipation of modifying this behaviour, add a test verifying the\nhandling of exclamation marks when looking up a commit \"by name\".\n\nSpecifically, as documented: '^{/!Message}' should fail, as this syntax\nis currently reserved; while '^{!!Message}' should search for a commit\nwhose message contains the string \"!Message\".\n\nSigned-off-by: Will Palmer <wmpalmer@gmail.com>\n---\n t/t1511-rev-parse-caret.sh | 19 ++++++++++++++++++-\n 1 file changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex 15973f2..0c46e5c 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -18,7 +18,14 @@ test_expect_success 'setup' '\n \tgit checkout master &&\n \techo modified >>a-blob &&\n \tgit add -u &&\n-\tgit commit -m Modified\n+\tgit commit -m Modified &&\n+\techo changed! >>a-blob &&\n+\tgit add -u &&\n+\tgit commit -m !Exp &&\n+\tgit branch expref &&\n+\techo changed >>a-blob &&\n+\tgit add -u &&\n+\tgit commit -m Changed\n '\n \n test_expect_success 'ref^{non-existent}' '\n@@ -77,4 +84,14 @@ test_expect_success 'ref^{/Initial}' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'ref^{/!Exp}' '\n+\ttest_must_fail git rev-parse master^{/!Exp}\n+'\n+\n+test_expect_success 'ref^{/!!Exp}' '\n+\tgit rev-parse expref >expected &&\n+\tgit rev-parse master^{/!!Exp} >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.3.0.rc1\n"},{"id":"262918","messageId":"1433364854-30088-3-git-send-email-wmpalmer@gmail.com","threadId":"39518","inReplyTo":"1433364854-30088-1-git-send-email-wmpalmer@gmail.com","subject":"[PATCH 2/2] object name: introduce '^{/!<negative pattern>}' notation","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2015-06-03T20:54:14Z","receivedAt":"2015-06-03T20:54:14Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"To name a commit, you can now say\n\n    $ git rev-parse HEAD^{/!foo}\n\nand it will return the hash of the first commit reachable from HEAD,\nwhose commit message does not contain \"foo\".\n\nSince the ability to reference a commit by \"name\" was introduced (way\nback in 1.5, in 364d3e6), with the across-all-refs syntax of ':/foo',\nthere has been a note in the documentation indicating that a leading\nexclamation mark was \"reserved for now\" (unless followed immediately be\nanother exclamation mark.)\n\nAt the time, this was sensible: we didn't get the '^{/foo}' flavour\nuntil sometime around 1.7.4 (41cd797) , so while a \"negative search\" was\na foreseeable feature, it wouldn't have made much sense to apply one\nacross all refs, as the result would have been essentially random.\n\nThese days, a negative pattern can make sense. In particular, if you tend\nto use a rebase-heavy workflow with many \"work in progress\" commits, it\nmay be useful to diff or rebase against the latest \"not work-in-progress\"\ncommit. That sort of thing now possible, via commands such as:\n\n    $ git rebase -i @^{/!^WIP}\n\nPerhaps notably, the \"special case\" for the empty pattern has been\nextended to handle the empty negative pattern - which never matches, to\ncontinue to ensure that an empty pattern never reaches the real regexp\ncode, as per notes in 4322842 \"get_sha1: handle special case $commit^{/}\"\n\nSigned-off-by: Will Palmer <wmpalmer@gmail.com>\n---\n Documentation/revisions.txt |  7 ++++---\n sha1_name.c                 | 22 ++++++++++++++++------\n t/t1511-rev-parse-caret.sh  | 32 +++++++++++++++++++++++++++++---\n 3 files changed, 49 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/revisions.txt b/Documentation/revisions.txt\nindex 0796118..6a6b8b9 100644\n--- a/Documentation/revisions.txt\n+++ b/Documentation/revisions.txt\n@@ -151,9 +151,10 @@ existing tag object.\n   A colon, followed by a slash, followed by a text, names\n   a commit whose commit message matches the specified regular expression.\n   This name returns the youngest matching commit which is\n-  reachable from any ref.  If the commit message starts with a\n-  '!' you have to repeat that;  the special sequence ':/!',\n-  followed by something else than '!', is reserved for now.\n+  reachable from any ref.  To name a commit whose commit message does not\n+  match the specified regular expression, begin the pattern-part with a\n+  '!', e.g. ':/!foo'. If the commit message you wish to match starts with\n+  a '!' you have to repeat that.\n   The regular expression can match any part of the commit message. To\n   match messages starting with a string, one can use e.g. ':/^foo'.\n \ndiff --git a/sha1_name.c b/sha1_name.c\nindex 46218ba..3d50dc9 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -737,11 +737,15 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n \n \t\t/*\n \t\t * $commit^{/}. Some regex implementation may reject.\n-\t\t * We don't need regex anyway. '' pattern always matches.\n+\t\t * We don't need regex anyway. '' pattern always matches,\n+\t\t * and '!' pattern never matches.\n \t\t */\n \t\tif (sp[1] == '}')\n \t\t\treturn 0;\n \n+\t\tif (sp[1] == '!' && sp[2] == '}')\n+\t\t\treturn -1;\n+\n \t\tprefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));\n \t\tcommit_list_insert((struct commit *)o, &list);\n \t\tret = get_sha1_oneline(prefix, sha1, list);\n@@ -825,8 +829,9 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l\n  * through history and returning the first commit whose message starts\n  * the given regular expression.\n  *\n- * For future extension, ':/!' is reserved. If you want to match a message\n- * beginning with a '!', you have to repeat the exclamation mark.\n+ * For negative-matching, prefix the pattern-part with a '!', like:\n+ * ':/!WIP'. If you want to match a message beginning with a literal\n+ * '!', you heave to repeat the exlamation mark.\n  */\n \n /* Remember to update object flag allocation in object.h */\n@@ -855,11 +860,16 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,\n {\n \tstruct commit_list *backup = NULL, *l;\n \tint found = 0;\n+\tint negative = 0;\n \tregex_t regex;\n \n \tif (prefix[0] == '!') {\n-\t\tif (prefix[1] != '!')\n-\t\t\tdie (\"Invalid search pattern: %s\", prefix);\n+\t\tif (prefix[1] != '!') {\n+\t\t\tnegative = 1;\n+\t\t} else if (prefix[1] == '!' && prefix[2] == '!') {\n+\t\t\tnegative = 1;\n+\t\t\tprefix++;\n+\t\t}\n \t\tprefix++;\n \t}\n \n@@ -880,7 +890,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,\n \t\t\tcontinue;\n \t\tbuf = get_commit_buffer(commit, NULL);\n \t\tp = strstr(buf, \"\\n\\n\");\n-\t\tmatches = p && !regexec(&regex, p + 2, 0, NULL, 0);\n+\t\tmatches = p && (negative ^ !regexec(&regex, p + 2, 0, NULL, 0));\n \t\tunuse_commit_buffer(commit, buf);\n \n \t\tif (matches) {\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex 0c46e5c..1d27aca 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -19,13 +19,17 @@ test_expect_success 'setup' '\n \techo modified >>a-blob &&\n \tgit add -u &&\n \tgit commit -m Modified &&\n+\tgit branch modref &&\n \techo changed! >>a-blob &&\n \tgit add -u &&\n \tgit commit -m !Exp &&\n \tgit branch expref &&\n \techo changed >>a-blob &&\n \tgit add -u &&\n-\tgit commit -m Changed\n+\tgit commit -m Changed &&\n+\techo changed-again >>a-blob &&\n+\tgit add -u &&\n+\tgit commit -m Changed-again\n '\n \n test_expect_success 'ref^{non-existent}' '\n@@ -84,8 +88,8 @@ test_expect_success 'ref^{/Initial}' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'ref^{/!Exp}' '\n-\ttest_must_fail git rev-parse master^{/!Exp}\n+test_expect_success 'ref^{/!}' '\n+\ttest_must_fail git rev-parse master^{/!}\n '\n \n test_expect_success 'ref^{/!!Exp}' '\n@@ -94,4 +98,26 @@ test_expect_success 'ref^{/!!Exp}' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'ref^{/!.}' '\n+\ttest_must_fail git rev-parse master^{/\\!.}\n+'\n+\n+test_expect_success 'ref^{/!non-existent}' '\n+\tgit rev-parse master >expected &&\n+\tgit rev-parse master^{/\\!non-existent} >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'ref^{/!Changed}' '\n+\tgit rev-parse expref >expected &&\n+\tgit rev-parse master^{/!Changed} >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'ref^{/!!!Exp}' '\n+\tgit rev-parse modref >expected &&\n+\tgit rev-parse expref^{/!!!Exp} >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.3.0.rc1\n"},{"id":"262922","messageId":"xmqqbngwwjbd.fsf@gitster.dls.corp.google.com","threadId":"39518","inReplyTo":"1433364854-30088-2-git-send-email-wmpalmer@gmail.com","subject":"Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-03T21:52:54Z","receivedAt":"2015-06-03T21:52:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Will Palmer <wmpalmer@gmail.com> writes:\n\n> Specifically, as documented: '^{/!Message}' should fail, as this syntax\n> is currently reserved; while '^{!!Message}' should search for a commit\n> whose message contains the string \"!Message\".\n\nThe /! sequence being reserved does not mean it was planned to be\nused only for a single thing in the future, though.\n\nThink of it as a syntax to introduce extended features, the first\nuse of which was this:\n\n\t/!!string\t-> find commit with \"!string\"\n\nThe above is just one \"feature\" that the reserved syntax allows,\nnamely, \"to find a string that begins with an exclamation mark\".\nThe anticipation is to use another feature introducer after \"/!\" to\nenhance the matching, so that we can keep enhancing the syntax.\n\ncf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477\n\nUsing \"/!Message\" to match commits that do not match Message\ndirectly goes against that extensivility design.\n\nWe need to always remind ourselves that our latest shiny new toy\nwill not be the final new feature.  There always will be need to add\nyet another new thing, and we need to keep the door open for them.\n\nPerhaps\n\n\t/!-string\t-> find commit without \"string\"\n\nor something?\n"},{"id":"262927","messageId":"CAAKF_uYrjBsVY8YOmRtMU8jB5rA57r+-N_KboqwWL3YRRqeKAg@mail.gmail.com","threadId":"39518","inReplyTo":"xmqqbngwwjbd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2015-06-03T22:44:59Z","receivedAt":"2015-06-03T22:44:59Z","isPatch":true,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Wed, Jun 3, 2015 at 10:52 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> The /! sequence being reserved does not mean it was planned to be\n> used only for a single thing in the future, though.\n>\n> (snip)\n>\n> cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477\n>\n\nThank you for that additional context, which I didn't see previously.\n\n> Using \"/!Message\" to match commits that do not match Message\n> directly goes against that extensivility design.\n>\n> We need to always remind ourselves that our latest shiny new toy\n> will not be the final new feature.  There always will be need to add\n> yet another new thing, and we need to keep the door open for them.\n>\n> Perhaps\n>\n>         /!-string       -> find commit without \"string\"\n>\n> or something?\n>\n\nWhat I'm thinking now is that \"@^{/foo}\" can be thought of as a\npotential \"shorthand-form\" of what could be \"@^{/!(m=foo)}\", in which\ncase \"@^{/!-foo}\" could similarly be thought of as a potential\nshorthand-form of what could be \"@^{/!(m-foo)}\".\n\nSo with that in mind, I agree that a syntax of \"@^{/!-foo}\" could indeed give\nme the results I'm looking for, while leaving room for the previously\nmentioned forms of future extension.\n\nI don't know if I consider those potential extensions to be commendable\nas a unified (and chain-able) syntax for finding revisions in the graph,\nor to be needless clutter which would only add \"yet another way to specify\nthe same thing\". I mean, I like the idea of being able to specify that\nI want \"The third parent of the first commit authored by Fred which is\nalso an ancestor of a commit which touched a file in the libraries\nsubdirectory\", it sounds like maybe it would be good to be able to do\nthat sort of thing without bringing xargs and shell expansion into the\npicture... but I certainly don't have a clue what it might be good for!\n\nIn any case, it sounds like we have a good way forward for this smaller\nchange, at least. I'll re-submit with the suggested syntax.\n"},{"id":"262928","messageId":"xmqq7frkwfmw.fsf@gitster.dls.corp.google.com","threadId":"39518","inReplyTo":"xmqqbngwwjbd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-03T23:12:23Z","receivedAt":"2015-06-03T23:12:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The anticipation is to use another feature introducer after \"/!\" to\n> enhance the matching, so that we can keep enhancing the syntax.\n>\n> cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477\n>\n> Using \"/!Message\" to match commits that do not match Message\n> directly goes against that extensivility design.\n>\n> We need to always remind ourselves that our latest shiny new toy\n> will not be the final new feature.  There always will be need to add\n> yet another new thing, and we need to keep the door open for them.\n>\n> Perhaps\n>\n> \t/!-string\t-> find commit without \"string\"\n>\n> or something?\n\nOf course, as I do not think it is something people would do\nregularly to look for a non-match, I do not necessarily think we\nneed a short-hand \"/!-string\".  Perhaps following the long-hand\nsyntax suggested in that old article, it may be sensible to start\nwith something more descriptive like\n\n\t/!(negative)string\n\nto look for a commit that does not say \"string\", without the\nshort-hand form.  Only after we see that people find the feature\nuseful and find the need to use it frequently (if it ever happens,\nthat is), we can introduce \"/!-string\" as a short-hand form as a\nfollow-up patch.\n\nThanks.\n"},{"id":"262964","messageId":"xmqqy4jzv1ry.fsf@gitster.dls.corp.google.com","threadId":"39518","inReplyTo":"CAAKF_uYrjBsVY8YOmRtMU8jB5rA57r+-N_KboqwWL3YRRqeKAg@mail.gmail.com","subject":"Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-04T17:09:21Z","receivedAt":"2015-06-04T17:09:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Will Palmer <wmpalmer@gmail.com> writes:\n\n> What I'm thinking now is that \"@^{/foo}\" can be thought of as a\n> potential \"shorthand-form\" of what could be \"@^{/!(m=foo)}\", in which\n> case \"@^{/!-foo}\" could similarly be thought of as a potential\n> shorthand-form of what could be \"@^{/!(m-foo)}\".\n\nAh, our messages crossed, it seems.  Yes, I think we are on the same\npage, and it is sensible to think of \"/!-string\" as a short-hand for\nthe more complete syntax that uses descriptive word, not mnemonic,\ne.g. \"/!(unmatch=string)\", that the old thread envisioned.\n\nI think it is OK (and probably preferrable) to start with only\n\"/!-string\" without the long-hand, as we do not know how multiple\nlong-hand instructions should interact with each other.\n\nThanks.\n"}]}