Re: [PATCH v2 2/2] object name: introduce '^{/!-negative pattern}' notation
On Mon, Jun 8, 2015 at 5:39 PM, Junio C Hamano gits...@pobox.com wrote:
Will Palmer wmpal...@gmail.com writes:
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index e0fe102..8a5983f 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
This probably belongs to the previous step, no?
As it isn't referenced until the negative tests, I didn't bother adding
it in the verify the way things are tests. Funny that it was mentioned,
as I *did* originally have it in the first commit, but I moved it to the
commit in which it was first used, so that it would be easier to notice.
+test_expect_success 'ref^{/!-}' '
+ test_must_fail git rev-parse master^{/!-}
+'
H, we must fail because...? We are looking for something that
does not contain an empty string, which by definition does not
exist.
Funny, but is correct ;-).
This is left-over from the original patch's logic, which included a
short-circuit to avoid an empty regex (as per 4322842 get_sha1: handle
special case $commit^{/})... which I now realise perhaps should
have been simply rephrased, rather than ommitted entirely.
I feel like adding something like:
8--
--- 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] == '-' sp[3] == '}')
+ 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);
--8
...would be the wrong place for this short-circuit check, in light of
discussion around extensibility; so, I'll see how it looks moving that
into get_sha1_oneline(...)
+test_expect_success 'ref^{/!-.}' '
+ test_must_fail git rev-parse master^{/!-.}
+'
Likewise. I however wonder if we catch a commit without any message
(which you probably have to craft with either commit-tree or
hash-object), but that falls into the curiosity not the
practicality category.
A commit with no message should indeed by returned by 'master^{/!-.}',
or at least, that is the intent. This test is only meant to cover the
result of there being no matching commit, however.
In summary: it looks like I'll be sending another one.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] test for '!' handling in rev-parse's named commits
In anticipation of extending this behaviour, add tests verifying the
handling of exclamation marks when looking up a commit by name.
Specifically, as documented: 'rev^{/!Message}' should fail, as the '!'
prefix is reserved; while 'rev^{!!Message}' should search for a commit
whose message contains the string !Message.
Signed-off-by: Will Palmer wmpal...@gmail.com
---
t/t1511-rev-parse-caret.sh | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index 15973f2..e0fe102 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,18 @@ 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}' '
+ git rev-parse expref expected
+ git rev-parse master^{/!!Exp} actual
+ test_cmp expected actual
+'
+
test_done
--
2.3.0.rc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] specify commit by negative pattern
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.
This is the second version of the patch series. The previous attempt
used the notation @^{/!WIP}, rather than @^{/!-WIP}, so the modifier
was the '!' character. Now, '!' is taken as an indicator that the
pattern is to be interpreted differently, and '-' is taken as an
indicator of how it is to be interpreted differently. This follows
recent discussion with Junio C Hamano gits...@pobox.com and much-less
recent discussion archived at:
http://thread.gmane.org/gmane.comp.version-control.git/40460/focus=40477
In summary, '!' is to be used as an escape hatch, for further
extension of the name commit by pattern functionality. Theorised future
extensions indicated things like what was to be searched,
e.g.: @^{/!(a=author)}. With only two interpretations of the '!'
leader, for now (including the '!!' literal notation), adding such a
verbose form, such as '@^{/!(negative)foo}', seemed inappropriate at this
time. In the event that such verbose forms are ever implemented, this new
form may act as a shorthand, for a basic case.
Will Palmer (2):
test for '!' handling in rev-parse's named commits
object name: introduce '^{/!-negative pattern}' notation
Documentation/revisions.txt | 11 +-
sha1_name.c | 20 -
t/t1511-rev-parse-caret.sh | 53 -
3 files changed, 73 insertions(+), 11 deletions(-)
--
2.3.0.rc1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] object name: introduce '^{/!-negative pattern}' notation
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. This is the opposite of the
existing rev^{/pattern} syntax.
The specific use-case this is intended for is to perform an operation,
excluding the most-recent commits containing a particular marker. For
example, if you tend to make work in progress commits, with messages
beginning with WIP, you work, then it could be useful to diff against
the most recent commit which was not a WIP commit. That sort of thing
now possible, via commands such as:
$ git diff @^{/!-^WIP}
The leader '/!-', rather than simply '/!', to denote a negative match,
is chosen to leave room for additional modifiers in the future.
Signed-off-by: Will Palmer wmpal...@gmail.com
---
Documentation/revisions.txt | 11 ++-
sha1_name.c | 20 +++-
t/t1511-rev-parse-caret.sh | 32 +++-
3 files changed, 52 insertions(+), 11 deletions(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index d85e303..0c84d4f 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -176,11 +176,12 @@ 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.
- The regular expression can match any part of the commit message. To
- match messages starting with a string, one can use e.g. ':/^foo'.
+ reachable from any ref. The regular expression can match any part of the
+ commit message. To match messages starting with a string, one can use
+ e.g. ':/^foo'. The special sequence ':/!' is reserved for modifiers to what
+ is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
+ literal '!' character, followed by 'foo'. Any other sequence beginning with
+ ':/!' is reserved for now.
'rev:path', e.g. 'HEAD:README', ':README', 'master:./README'::
A suffix ':' followed by a path names the blob or tree
diff --git a/sha1_name.c b/sha1_name.c
index e57513e..82de2db 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -838,8 +838,12 @@ 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 '!-', like: ':/!-WIP'.
+ *
+ * For a literal '!' character at the beginning of a pattern, you have to
repeat
+ * that, like: ':/!!foo'
+ *
+ * For future extension, all other sequences beginning with ':/!' are reserved.
*/
/* Remember to update object flag allocation in object.h */
@@ -868,12 +872,18 @@ 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);
prefix++;
+
+ if (prefix[0] == '-') {
+ prefix++;
+ negative = 1;
+ } else if (prefix[0] != '!') {
+ die (Invalid search pattern: %s, prefix);
+ }
}
if (regcomp(regex, prefix, REG_EXTENDED))
@@ -893,7 +903,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 e0fe102..8a5983f 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}' '
@@ -98,4 +102,30 @@ test_expect_success 'ref
[PATCH 0/2] specify commit by negative pattern
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] object name: introduce '^{/!negative pattern}' notation
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 wmpal...@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
[PATCH 1/2] test for '!' handling in rev-parse's named commits
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 wmpal...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test for '!' handling in rev-parse's named commits
On Wed, Jun 3, 2015 at 10:52 PM, Junio C Hamano gits...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
