[PATCH 0/2] specify commit by negative pattern

[PATCH 0/2] specify commit by negative pattern

[Will Palmer]

add support for negative pattern matching in @^{/<pattern>} style
revision specifiers. So now you can find the first commit whose message
doesn't match a pattern, complementing the existing positive matching.
e.g.:

    $ git rebase -i @^{/!^WIP}

My use-case is in having a "work, work, work, rebase, push"-style
workflow, which generates a lot of "WIP foo" commits. While rebasing is
usually handled via "git rebase -i origin/master", occasionally I will
already have several "good, but not yet ready to push" commits hanging
around while I finish work on related commits. In these situations, the
ability to quickly "git diff @^{/!^WIP}" to get an overview of all
changes "since the last one I was happy with", can be useful.

Reading through the history of this type of revision specifier, it feels
like a negative match was always thought of as potentially useful
someday, but didn't fit well with the original patch's limitations
(namely: always searching across all refs).

Will Palmer (2):
  test for '!' handling in rev-parse's named commits
  object name: introduce '^{/!<negative pattern>}' notation

 Documentation/revisions.txt |  7 ++++---
 sha1_name.c                 | 22 ++++++++++++++++------
 t/t1511-rev-parse-caret.sh  | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 10 deletions(-)

-- 
2.3.0.rc1

[PATCH 1/2] test for '!' handling in rev-parse's named commits

[Will Palmer]

In anticipation of modifying this behaviour, add a test verifying the
handling of exclamation marks when looking up a commit "by name".

Specifically, as documented: '^{/!Message}' should fail, as this syntax
is currently reserved; while '^{!!Message}' should search for a commit
whose message contains the string "!Message".

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 t/t1511-rev-parse-caret.sh | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index 15973f2..0c46e5c 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -18,7 +18,14 @@ test_expect_success 'setup' '
 	git checkout master &&
 	echo modified >>a-blob &&
 	git add -u &&
-	git commit -m Modified
+	git commit -m Modified &&
+	echo changed! >>a-blob &&
+	git add -u &&
+	git commit -m !Exp &&
+	git branch expref &&
+	echo changed >>a-blob &&
+	git add -u &&
+	git commit -m Changed
 '
 
 test_expect_success 'ref^{non-existent}' '
@@ -77,4 +84,14 @@ test_expect_success 'ref^{/Initial}' '
 	test_cmp expected actual
 '
 
+test_expect_success 'ref^{/!Exp}' '
+	test_must_fail git rev-parse master^{/!Exp}
+'
+
+test_expect_success 'ref^{/!!Exp}' '
+	git rev-parse expref >expected &&
+	git rev-parse master^{/!!Exp} >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.3.0.rc1

Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits

[Junio C Hamano]

Will Palmer <wmpalmer@gmail.com> writes:

> Specifically, as documented: '^{/!Message}' should fail, as this syntax
> is currently reserved; while '^{!!Message}' should search for a commit
> whose message contains the string "!Message".

The /! sequence being reserved does not mean it was planned to be
used only for a single thing in the future, though.

Think of it as a syntax to introduce extended features, the first
use of which was this:

	/!!string	-> find commit with "!string"

The above is just one "feature" that the reserved syntax allows,
namely, "to find a string that begins with an exclamation mark".
The anticipation is to use another feature introducer after "/!" to
enhance the matching, so that we can keep enhancing the syntax.

cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477

Using "/!Message" to match commits that do not match Message
directly goes against that extensivility design.

We need to always remind ourselves that our latest shiny new toy
will not be the final new feature.  There always will be need to add
yet another new thing, and we need to keep the door open for them.

Perhaps

	/!-string	-> find commit without "string"

or something?

Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits

[Will Palmer]

On Wed, Jun 3, 2015 at 10:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The /! sequence being reserved does not mean it was planned to be
> used only for a single thing in the future, though.
>
> (snip)
>
> cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477
>

Thank you for that additional context, which I didn't see previously.

> Using "/!Message" to match commits that do not match Message
> directly goes against that extensivility design.
>
> We need to always remind ourselves that our latest shiny new toy
> will not be the final new feature.  There always will be need to add
> yet another new thing, and we need to keep the door open for them.
>
> Perhaps
>
>         /!-string       -> find commit without "string"
>
> or something?
>

What I'm thinking now is that "@^{/foo}" can be thought of as a
potential "shorthand-form" of what could be "@^{/!(m=foo)}", in which
case "@^{/!-foo}" could similarly be thought of as a potential
shorthand-form of what could be "@^{/!(m-foo)}".

So with that in mind, I agree that a syntax of "@^{/!-foo}" could indeed give
me the results I'm looking for, while leaving room for the previously
mentioned forms of future extension.

I don't know if I consider those potential extensions to be commendable
as a unified (and chain-able) syntax for finding revisions in the graph,
or to be needless clutter which would only add "yet another way to specify
the same thing". I mean, I like the idea of being able to specify that
I want "The third parent of the first commit authored by Fred which is
also an ancestor of a commit which touched a file in the libraries
subdirectory", it sounds like maybe it would be good to be able to do
that sort of thing without bringing xargs and shell expansion into the
picture... but I certainly don't have a clue what it might be good for!

In any case, it sounds like we have a good way forward for this smaller
change, at least. I'll re-submit with the suggested syntax.

Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits

[Junio C Hamano]

Will Palmer <wmpalmer@gmail.com> writes:

> What I'm thinking now is that "@^{/foo}" can be thought of as a
> potential "shorthand-form" of what could be "@^{/!(m=foo)}", in which
> case "@^{/!-foo}" could similarly be thought of as a potential
> shorthand-form of what could be "@^{/!(m-foo)}".

Ah, our messages crossed, it seems.  Yes, I think we are on the same
page, and it is sensible to think of "/!-string" as a short-hand for
the more complete syntax that uses descriptive word, not mnemonic,
e.g. "/!(unmatch=string)", that the old thread envisioned.

I think it is OK (and probably preferrable) to start with only
"/!-string" without the long-hand, as we do not know how multiple
long-hand instructions should interact with each other.

Thanks.

Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> The anticipation is to use another feature introducer after "/!" to
> enhance the matching, so that we can keep enhancing the syntax.
>
> cf. http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477
>
> Using "/!Message" to match commits that do not match Message
> directly goes against that extensivility design.
>
> We need to always remind ourselves that our latest shiny new toy
> will not be the final new feature.  There always will be need to add
> yet another new thing, and we need to keep the door open for them.
>
> Perhaps
>
> 	/!-string	-> find commit without "string"
>
> or something?

Of course, as I do not think it is something people would do
regularly to look for a non-match, I do not necessarily think we
need a short-hand "/!-string".  Perhaps following the long-hand
syntax suggested in that old article, it may be sensible to start
with something more descriptive like

	/!(negative)string

to look for a commit that does not say "string", without the
short-hand form.  Only after we see that people find the feature
useful and find the need to use it frequently (if it ever happens,
that is), we can introduce "/!-string" as a short-hand form as a
follow-up patch.

Thanks.

[PATCH 2/2] object name: introduce '^{/!<negative pattern>}' notation

[Will Palmer]

To name a commit, you can now say

    $ git rev-parse HEAD^{/!foo}

and it will return the hash of the first commit reachable from HEAD,
whose commit message does not contain "foo".

Since the ability to reference a commit by "name" was introduced (way
back in 1.5, in 364d3e6), with the across-all-refs syntax of ':/foo',
there has been a note in the documentation indicating that a leading
exclamation mark was "reserved for now" (unless followed immediately be
another exclamation mark.)

At the time, this was sensible: we didn't get the '^{/foo}' flavour
until sometime around 1.7.4 (41cd797) , so while a "negative search" was
a foreseeable feature, it wouldn't have made much sense to apply one
across all refs, as the result would have been essentially random.

These days, a negative pattern can make sense. In particular, if you tend
to use a rebase-heavy workflow with many "work in progress" commits, it
may be useful to diff or rebase against the latest "not work-in-progress"
commit. That sort of thing now possible, via commands such as:

    $ git rebase -i @^{/!^WIP}

Perhaps notably, the "special case" for the empty pattern has been
extended to handle the empty negative pattern - which never matches, to
continue to ensure that an empty pattern never reaches the real regexp
code, as per notes in 4322842 "get_sha1: handle special case $commit^{/}"

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 Documentation/revisions.txt |  7 ++++---
 sha1_name.c                 | 22 ++++++++++++++++------
 t/t1511-rev-parse-caret.sh  | 32 +++++++++++++++++++++++++++++---
 3 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 0796118..6a6b8b9 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -151,9 +151,10 @@ existing tag object.
   A colon, followed by a slash, followed by a text, names
   a commit whose commit message matches the specified regular expression.
   This name returns the youngest matching commit which is
-  reachable from any ref.  If the commit message starts with a
-  '!' you have to repeat that;  the special sequence ':/!',
-  followed by something else than '!', is reserved for now.
+  reachable from any ref.  To name a commit whose commit message does not
+  match the specified regular expression, begin the pattern-part with a
+  '!', e.g. ':/!foo'. If the commit message you wish to match starts with
+  a '!' you have to repeat that.
   The regular expression can match any part of the commit message. To
   match messages starting with a string, one can use e.g. ':/^foo'.
 
diff --git a/sha1_name.c b/sha1_name.c
index 46218ba..3d50dc9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -737,11 +737,15 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
 
 		/*
 		 * $commit^{/}. Some regex implementation may reject.
-		 * We don't need regex anyway. '' pattern always matches.
+		 * We don't need regex anyway. '' pattern always matches,
+		 * and '!' pattern never matches.
 		 */
 		if (sp[1] == '}')
 			return 0;
 
+		if (sp[1] == '!' && sp[2] == '}')
+			return -1;
+
 		prefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));
 		commit_list_insert((struct commit *)o, &list);
 		ret = get_sha1_oneline(prefix, sha1, list);
@@ -825,8 +829,9 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l
  * through history and returning the first commit whose message starts
  * the given regular expression.
  *
- * For future extension, ':/!' is reserved. If you want to match a message
- * beginning with a '!', you have to repeat the exclamation mark.
+ * For negative-matching, prefix the pattern-part with a '!', like:
+ * ':/!WIP'. If you want to match a message beginning with a literal
+ * '!', you heave to repeat the exlamation mark.
  */
 
 /* Remember to update object flag allocation in object.h */
@@ -855,11 +860,16 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,
 {
 	struct commit_list *backup = NULL, *l;
 	int found = 0;
+	int negative = 0;
 	regex_t regex;
 
 	if (prefix[0] == '!') {
-		if (prefix[1] != '!')
-			die ("Invalid search pattern: %s", prefix);
+		if (prefix[1] != '!') {
+			negative = 1;
+		} else if (prefix[1] == '!' && prefix[2] == '!') {
+			negative = 1;
+			prefix++;
+		}
 		prefix++;
 	}
 
@@ -880,7 +890,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,
 			continue;
 		buf = get_commit_buffer(commit, NULL);
 		p = strstr(buf, "\n\n");
-		matches = p && !regexec(&regex, p + 2, 0, NULL, 0);
+		matches = p && (negative ^ !regexec(&regex, p + 2, 0, NULL, 0));
 		unuse_commit_buffer(commit, buf);
 
 		if (matches) {
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index 0c46e5c..1d27aca 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -19,13 +19,17 @@ test_expect_success 'setup' '
 	echo modified >>a-blob &&
 	git add -u &&
 	git commit -m Modified &&
+	git branch modref &&
 	echo changed! >>a-blob &&
 	git add -u &&
 	git commit -m !Exp &&
 	git branch expref &&
 	echo changed >>a-blob &&
 	git add -u &&
-	git commit -m Changed
+	git commit -m Changed &&
+	echo changed-again >>a-blob &&
+	git add -u &&
+	git commit -m Changed-again
 '
 
 test_expect_success 'ref^{non-existent}' '
@@ -84,8 +88,8 @@ test_expect_success 'ref^{/Initial}' '
 	test_cmp expected actual
 '
 
-test_expect_success 'ref^{/!Exp}' '
-	test_must_fail git rev-parse master^{/!Exp}
+test_expect_success 'ref^{/!}' '
+	test_must_fail git rev-parse master^{/!}
 '
 
 test_expect_success 'ref^{/!!Exp}' '
@@ -94,4 +98,26 @@ test_expect_success 'ref^{/!!Exp}' '
 	test_cmp expected actual
 '
 
+test_expect_success 'ref^{/!.}' '
+	test_must_fail git rev-parse master^{/\!.}
+'
+
+test_expect_success 'ref^{/!non-existent}' '
+	git rev-parse master >expected &&
+	git rev-parse master^{/\!non-existent} >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'ref^{/!Changed}' '
+	git rev-parse expref >expected &&
+	git rev-parse master^{/!Changed} >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'ref^{/!!!Exp}' '
+	git rev-parse modref >expected &&
+	git rev-parse expref^{/!!!Exp} >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.3.0.rc1