[PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyễn Thái Ngọc Duy]

Let's start off from where the previous discussion [1] stopped. People
seem to agree ref^{/regex} is a good choice. But we have not come to
conclusion how to specify the count yet. Possible suggestions are

 - ref^{/foo}2
 - ref^{2/foo}
 - ref^{:2/foo}
 - ref^{2nd/foo}

For whatever syntax chosen, :/ should benefit too. I notice that :/!
is reserved for future use. Perhaps :/!2/regex is not too cryptic?

I'd also like to do case-insensitive regex, by the way. :/!2i/regex
looks a bit ugly.

[1] http://mid.gmane.org/9D675671-693D-4B59-AF2A-0EFE4C537362@sb.org

Nguyễn Thái Ngọc Duy (2):
  get_sha1_oneline: allow to input commit_list
  get_sha1: support ref^{/regex} syntax

 Documentation/revisions.txt |    7 ++++++
 sha1_name.c                 |   45 ++++++++++++++++++++++++++++++++----------
 2 files changed, 41 insertions(+), 11 deletions(-)

-- 
1.7.3.2.316.gda8b3

[PATCH 1/2] get_sha1_oneline: allow to input commit_list

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 sha1_name.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 2c3a5fb..f4ccdc5 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -686,13 +686,14 @@ static int handle_one_ref(const char *path,
 	if (object->type != OBJ_COMMIT)
 		return 0;
 	insert_by_date((struct commit *)object, list);
-	object->flags |= ONELINE_SEEN;
 	return 0;
 }
 
-static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
+static int get_sha1_oneline(const char *prefix,
+			    unsigned char *sha1,
+			    struct commit_list *list_)
 {
-	struct commit_list *list = NULL, *backup = NULL, *l;
+	struct commit_list *list = list_, *backup = NULL, *l;
 	int retval = -1;
 	char *temp_commit_buffer = NULL;
 	regex_t regex;
@@ -706,9 +707,12 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	if (regcomp(&regex, prefix, REG_EXTENDED))
 		die("Invalid search pattern: %s", prefix);
 
-	for_each_ref(handle_one_ref, &list);
-	for (l = list; l; l = l->next)
+	if (!list)
+		for_each_ref(handle_one_ref, &list);
+	for (l = list; l; l = l->next) {
 		commit_list_insert(l->item, &backup);
+		l->item->object.flags |= ONELINE_SEEN;
+	}
 	while (list) {
 		char *p;
 		struct commit *commit;
@@ -737,7 +741,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	}
 	regfree(&regex);
 	free(temp_commit_buffer);
-	free_commit_list(list);
+	if (!list_)
+		free_commit_list(list);
 	for (l = backup; l; l = l->next)
 		clear_commit_marks(l->item, ONELINE_SEEN);
 	return retval;
@@ -1090,7 +1095,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 		int pos;
 		if (namelen > 2 && name[1] == '/')
 			/* don't need mode for commit */
-			return get_sha1_oneline(name + 2, sha1);
+			return get_sha1_oneline(name + 2, sha1, NULL);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
 		    name[1] < '0' || '3' < name[1])
-- 
1.7.3.2.316.gda8b3

Re: [PATCH 1/2] get_sha1_oneline: allow to input commit_list

[Thiago Farina]

2010/12/8 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> -static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
> +static int get_sha1_oneline(const char *prefix,
> +                           unsigned char *sha1,
> +                           struct commit_list *list_)
>  {

micronit: can we have a better name for |list_|, the suffix _ is very
ugly and uncommon :(

[PATCH 2/2] get_sha1: support ref^{/regex} syntax

[Nguyễn Thái Ngọc Duy]

This works like :/ syntax, but only limited to one ref.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/revisions.txt |    7 +++++++
 sha1_name.c                 |   26 ++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 3d4b79c..fbe6245 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -106,6 +106,13 @@ the `$GIT_DIR/refs` directory or from the `$GIT_DIR/packed-refs` file.
   and dereference the tag recursively until a non-tag object is
   found.
 
+* A suffix '{caret}' to a revision parameter followed by a brace
+  pair that contains a text led by a slash (e.g. `HEAD^{/fix nasty bug}`):
+  this names a commit whose commit message matches the specified
+  regular expression. This name returns the youngest matching commit
+  which is reachable from the dereferenced commit. The leading '!'
+  in the text is treated especially like in `:/` syntax below.
+
 * A colon, followed by a slash, followed by a text (e.g. `:/fix nasty bug`): this names
   a commit whose commit message matches the specified regular expression.
   This name returns the youngest matching commit which is
diff --git a/sha1_name.c b/sha1_name.c
index f4ccdc5..00e52b0 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -527,6 +527,7 @@ struct object *peel_to_type(const char *name, int namelen,
 	}
 }
 
+static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);
 static int peel_onion(const char *name, int len, unsigned char *sha1)
 {
 	unsigned char outer[20];
@@ -562,6 +563,11 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
 		expected_type = OBJ_BLOB;
 	else if (sp[0] == '}')
 		expected_type = OBJ_NONE;
+	else if (sp[0] == '/') {
+		if (sp[1] == '}')
+			return -1;
+		expected_type = OBJ_COMMIT;
+	}
 	else
 		return -1;
 
@@ -584,11 +590,23 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
 		 * barf.
 		 */
 		o = peel_to_type(name, len, o, expected_type);
-		if (o) {
-			hashcpy(sha1, o->sha1);
-			return 0;
+		if (!o)
+			return -1;
+
+		hashcpy(sha1, o->sha1);
+		if (sp[0] == '/') { /* ^{/foo} */
+			struct commit_list *list = NULL;
+			char *prefix;
+			int ret;
+
+			commit_list_insert((struct commit *)o, &list);
+			prefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));
+			ret = get_sha1_oneline(prefix, sha1, list);
+			free(prefix);
+			free_commit_list(list);
+			return ret;
 		}
-		return -1;
+		return 0;
 	}
 	return 0;
 }
-- 
1.7.3.2.316.gda8b3

Re: [PATCH 2/2] get_sha1: support ref^{/regex} syntax

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> diff --git a/sha1_name.c b/sha1_name.c
> index f4ccdc5..00e52b0 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -562,6 +563,11 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
>  		expected_type = OBJ_BLOB;
>  	else if (sp[0] == '}')
>  		expected_type = OBJ_NONE;
> +	else if (sp[0] == '/') {
> +		if (sp[1] == '}')
> +			return -1;

Why?  $commit^{/} may be a no-op but I do not see a strong reason to
waste extra two lines to forbid it.

> @@ -584,11 +590,23 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
>  		 * barf.
>  		 */
>  		o = peel_to_type(name, len, o, expected_type);
> -		if (o) {
> -			hashcpy(sha1, o->sha1);
> -			return 0;
> +		if (!o)
> +			return -1;

I can see you are trying to reduce nesting of

        if (o) {
		do true thing
                return 0
	}
        return -1;

but then we should apply the same to outer "if (!expected_type) ... else",
too, to unnest the "else" clause by returning from the true branch of that
"if".

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jonathan Nieder]

Nguyễn Thái Ngọc Duy wrote:

> Let's start off from where the previous discussion [1] stopped. People
> seem to agree ref^{/regex} is a good choice. But we have not come to
> conclusion how to specify the count yet. Possible suggestions are
>
>  - ref^{/foo}2
>  - ref^{2/foo}
>  - ref^{:2/foo}
>  - ref^{2nd/foo}

How about

	ref^{/foo}^^{/foo}

?

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

Dnia środa 8. grudnia 2010 19:06, Jonathan Nieder napisał:
> Nguyễn Thái Ngọc Duy wrote:
> 
> > Let's start off from where the previous discussion [1] stopped. People
> > seem to agree ref^{/regex} is a good choice. But we have not come to
> > conclusion how to specify the count yet. Possible suggestions are
> >
> >  - ref^{/foo}2
> >  - ref^{2/foo}
> >  - ref^{:2/foo}
> >  - ref^{2nd/foo}
> 
> How about
> 
> 	ref^{/foo}^^{/foo}
> 
> ?

I'll assume that there is invisible ";)" emoticon here.


First, it would be ref^{/foo}^@^{/foo}, otherwise you would follow only
first parent.

Second, consider ref^{:nth(10)/foo} in your workaround...

;-)

-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyen Thai Ngoc Duy]

On Thu, Dec 9, 2010 at 2:51 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Dnia środa 8. grudnia 2010 19:06, Jonathan Nieder napisał:
>> Nguyễn Thái Ngọc Duy wrote:
>>
>> > Let's start off from where the previous discussion [1] stopped. People
>> > seem to agree ref^{/regex} is a good choice. But we have not come to
>> > conclusion how to specify the count yet. Possible suggestions are
>> >
>> >  - ref^{/foo}2
>> >  - ref^{2/foo}
>> >  - ref^{:2/foo}
>> >  - ref^{2nd/foo}
>>
>> How about
>>
>>       ref^{/foo}^^{/foo}
>>
>> ?
>
> I'll assume that there is invisible ";)" emoticon here.
>
>
> First, it would be ref^{/foo}^@^{/foo}, otherwise you would follow only
> first parent.
>
> Second, consider ref^{:nth(10)/foo} in your workaround...

Maybe we should generalize this to apply to all operators. Currently
foo~3 is expanded to foo^^^. How about ~~X (or xN) denote repeat the
last operator N times? For example, HEAD^2x3 => HEAD^2^2^2,
HEAD^{/foo}x3 => HEAD^{/foo}^{/foo}^{/foo}.
-- 
Duy

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

Nguyen Thai Ngoc Duy wrote:
> On Thu, Dec 9, 2010 at 2:51 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> Dnia środa 8. grudnia 2010 19:06, Jonathan Nieder napisał:
>>> Nguyễn Thái Ngọc Duy wrote:
>>>
>>>> Let's start off from where the previous discussion [1] stopped. People
>>>> seem to agree ref^{/regex} is a good choice. But we have not come to
>>>> conclusion how to specify the count yet. Possible suggestions are
>>>>
>>>>  - ref^{/foo}2
>>>>  - ref^{2/foo}
>>>>  - ref^{:2/foo}
>>>>  - ref^{2nd/foo}
>>>
>>> How about
>>>
>>>       ref^{/foo}^^{/foo}
>>>
>>> ?
>>
>> I'll assume that there is invisible ";)" emoticon here.
>>
>>
>> First, it would be ref^{/foo}^@^{/foo}, otherwise you would follow only
>> first parent.
>>
>> Second, consider ref^{:nth(10)/foo} in your workaround...
> 
> Maybe we should generalize this to apply to all operators. Currently
> foo~3 is expanded to foo^^^. How about ~~X (or xN) denote repeat the
> last operator N times? For example, HEAD^2x3 => HEAD^2^2^2,
> HEAD^{/foo}x3 => HEAD^{/foo}^{/foo}^{/foo}.

Unless you allow grouping, it wouldn't help in the case of ^{/foo},
because ^{/foo} is idempotent.  HEAD^{/foo} finds first commit that
contains "foo", and HEAD^{/foo}^{/foo} finds first commit containing
"foo" starting from *and including* first commit from HEAD containing
"foo" - which is HEAD^{/foo}

  HEAD^{/foo}^{/foo} === HEAD^{/foo}

You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.

-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jonathan Nieder]

Jakub Narebski wrote:

> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.

That said, does ^2x500 really do something meaningful that a person
would ever need?  I like the

	^{:nth(3)/foo}

syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
already-defined modifiers to implement when they prove useful, known
already to a certain subset of the audience and proven useful already
in a different context.

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Dec 8, 2010, at 5:59 PM, Jonathan Nieder wrote:

> Jakub Narebski wrote:
> 
>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
> 
> That said, does ^2x500 really do something meaningful that a person
> would ever need?  I like the
> 
> 	^{:nth(3)/foo}
> 
> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
> already-defined modifiers to implement when they prove useful, known
> already to a certain subset of the audience and proven useful already
> in a different context.

I like the ^{:nth(3)/foo} syntax as well. Though I'm not familiar with Perl 6,
this does have the benefit of being fairly obvious to the reader as to what it
means.

-Kevin Ballard

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyen Thai Ngoc Duy]

On Thu, Dec 9, 2010 at 9:02 AM, Kevin Ballard <kevin@sb.org> wrote:
> On Dec 8, 2010, at 5:59 PM, Jonathan Nieder wrote:
>
>> Jakub Narebski wrote:
>>
>>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>>
>> That said, does ^2x500 really do something meaningful that a person
>> would ever need?  I like the
>>
>>       ^{:nth(3)/foo}
>>
>> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
>> already-defined modifiers to implement when they prove useful, known
>> already to a certain subset of the audience and proven useful already
>> in a different context.
>
> I like the ^{:nth(3)/foo} syntax as well. Though I'm not familiar with Perl 6,
> this does have the benefit of being fairly obvious to the reader as to what it
> means.

OK so :nth(3)/foo for all branches?
-- 
Duy

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> OK so :nth(3)/foo for all branches?

That steals namespace from "the path 'nth(3)/foo' in the index".  But
is "the third instance of foo in all branches" something that needs to
be possible to say?  Branches do not have a well defined order,
anyway.  A command to list all commits with "foo" in the subject
like

	git log --oneline --grep-subject=foo

sounds more useful (assuming --grep=foo yields too many false
positives).

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Jakub Narebski wrote:
>
>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>
> That said, does ^2x500 really do something meaningful that a person
> would ever need?  I like the
>
> 	^{:nth(3)/foo}
>
> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
> already-defined modifiers to implement when they prove useful,...

Can you explain what the colon in "$commit^{:nth(3)/foo}" is doing?  

Are we declaring anything that begins with ':' is a magic inside ^{...}
construct?

I do not think nth($n) without specifying where to start (iow, what the
current ":/foo" implementation does but with "three levels deep") makes
any sense, but because the main point of your argument is that we can have
modifies other than nth($n) that may make sense in such a context, I would
want to make sure anything we come up with is extensible to that syntax.

On the "starting from any ref" front, I think "!" (as in ":/!some magic")
was the introducer we reserved for such a magic some time ago, so perhaps
on the "starting from this commit" side, "^{!magic/foo}" may be more
appropriate?

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Thu, 9 Dec 2010, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Jakub Narebski wrote:
>>
>>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>>
>> That said, does ^2x500 really do something meaningful that a person
>> would ever need?  I like the
>>
>> 	^{:nth(3)/foo}
>>
>> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
>> already-defined modifiers to implement when they prove useful,...
> 
> Can you explain what the colon in "$commit^{:nth(3)/foo}" is doing?  
> 
> Are we declaring anything that begins with ':' is a magic inside ^{...}
> construct?

The idea is to follow Perl 6 rule of combining modifiers "Every modifier
must start with its own colon."
  http://perlcabal.org/syn/S05.html#Modifiers

This allow for easy combining modifiers without introducing special rules,
for example ^{:g:i/foo} would return all matches (:g == :global) instead
of only the first, ignoring case (:i == :ignorecase).
 
> I do not think nth($n) without specifying where to start (iow, what the
> current ":/foo" implementation does but with "three levels deep") makes
> any sense, but because the main point of your argument is that we can have
> modifies other than nth($n) that may make sense in such a context, I would
> want to make sure anything we come up with is extensible to that syntax.

Hmmm... I haven't thought about it.  I guess that ^{:nth(N)/foo} should
return Nth element in the list that ^{:g/foo} / --grep=foo would return.

This way one could use

  $ git log pu@{1}..pu --merges --grep="Merge branch 'nd/"

to find all new Nguyen contributions in 'pu', and

  $ git log pu..pu^{:2nd/Merge branch 'nd/}^2

to view 2nd (2nd == nth(2)) such branch on the list.

> 
> On the "starting from any ref" front, I think "!" (as in ":/!some magic")
> was the introducer we reserved for such a magic some time ago, so perhaps
> on the "starting from this commit" side, "^{!magic/foo}" may be more
> appropriate?

I'd like to deprecate ":/foo" in favor of "--all ^{/foo}", or even special
case it to "^{/foo}" (if not attached to ref, ^{(:<modifier>)*/<regexp>}
acts on positive revs replacing them).

But ^{/foo} might be overengineered...
-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyen Thai Ngoc Duy]

On Thu, Dec 9, 2010 at 1:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Jakub Narebski wrote:
>>
>>> You would need HEAD{^{/foo}^@}x3, or use special rule that HEAD^{/foo}x2
>>> means really HEAD^{/foo}^@^{/foo}, with ^@ used to join them.
>>
>> That said, does ^2x500 really do something meaningful that a person
>> would ever need?  I like the
>>
>>       ^{:nth(3)/foo}
>>
>> syntax because perl6 supports m:nth(3)/foo/, suggesting a menu of
>> already-defined modifiers to implement when they prove useful,...
>
> Can you explain what the colon in "$commit^{:nth(3)/foo}" is doing?
>
> Are we declaring anything that begins with ':' is a magic inside ^{...}
> construct?
>
> I do not think nth($n) without specifying where to start (iow, what the
> current ":/foo" implementation does but with "three levels deep") makes
> any sense, but because the main point of your argument is that we can have
> modifies other than nth($n) that may make sense in such a context, I would
> want to make sure anything we come up with is extensible to that syntax.

There's also another similar operation: note search. I don't think we
need to invent another syntax for note search, a modifier for ^{/foo}
may be a good choice.

> On the "starting from any ref" front, I think "!" (as in ":/!some magic")
> was the introducer we reserved for such a magic some time ago, so perhaps
> on the "starting from this commit" side, "^{!magic/foo}" may be more
> appropriate?

Can we use ! modifier for other ^{} too? What I have in mind is how to
say ^{commit} that has two parents. Or even better, "search from the
given tip for a commit that has two parents and the commit message
matches 'foo'". Hmm.. too complex. Perhaps "^{grep: <grep arguments>}"
that pulls the whole git-grep functionality in.
-- 
Duy

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> Can we use ! modifier for other ^{} too? What I have in mind is how to
> say ^{commit} that has two parents. Or even better, "search from the
> given tip for a commit that has two parents and the commit message
> matches 'foo'". Hmm.. too complex. Perhaps "^{grep: <grep arguments>}"
> that pulls the whole git-grep functionality in.

My thoughts, in no particular order:

 - '!' can be a pain in the neck to supply on the bash command line.
   Single quotes and backslash quoting work while double quotes do
   not, unless 'set +H' has been run.  But that's not a huge deal and
   arguably it's a bash misfeature.

 - What is the intended use for this family of modifiers?  I sort
   of understand ^{:i/... } for people that forget what case they
   have used, but why the :nth and others?

 - Why do we have to carve out the namespace right away, anyway?  If
   we just ^{/... } for the ordinary "start here" search, that leaves
   room for anything after the { other than "/<pattern>" and a few
   fixed strings like "upstream", "tree", etc, right?  It might be
   easier to anticipate what syntax will be useful when there is
   functionality to go with it.

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Fri, 10 Dec 2010 20:03, Jonathan Nieder wrote:
> Nguyen Thai Ngoc Duy wrote:
> 
> > Can we use ! modifier for other ^{} too? What I have in mind is how to
> > say ^{commit} that has two parents. Or even better, "search from the
> > given tip for a commit that has two parents and the commit message
> > matches 'foo'". Hmm.. too complex. Perhaps "^{grep: <grep arguments>}"
> > that pulls the whole git-grep functionality in.
> 
> My thoughts, in no particular order:
> 
>  - '!' can be a pain in the neck to supply on the bash command line.
>    Single quotes and backslash quoting work while double quotes do
>    not, unless 'set +H' has been run.  But that's not a huge deal and
>    arguably it's a bash misfeature.
> 
>  - What is the intended use for this family of modifiers?  I sort
>    of understand ^{:i/... } for people that forget what case they
>    have used, but why the :nth and others?

* :nth(N) and :g, to show nth match and all matches, respectively; might
  be not necessary as it impinges a bit on --grep=<regexp> domain
* :N / :notesRef to search also through attached notes

If we was possible, perhaps also

* :b / :basechar to ignore accents and other marks (according to commit
  encoding).


-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Dec 10, 2010, at 11:03 AM, Jonathan Nieder wrote:

> - What is the intended use for this family of modifiers?  I sort
>   of understand ^{:i/... } for people that forget what case they
>   have used, but why the :nth and others?

In my particular case, I was glancing through the logs, and I wanted to grab
the second branch that someone else had made that was merged into pu. I would
have loved to be able to run something like

  git merge origin/pu^{:nth(2)/nd/}

While we're speaking of modifiers, could we use one that says "only search
the first parent hierarchy", e.g. something equivalent to git log's --first-parent
flag?

-Kevin Ballard

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jeff King]

On Fri, Dec 10, 2010 at 01:21:15PM -0800, Kevin Ballard wrote:

> On Dec 10, 2010, at 11:03 AM, Jonathan Nieder wrote:
> 
> > - What is the intended use for this family of modifiers?  I sort
> >   of understand ^{:i/... } for people that forget what case they
> >   have used, but why the :nth and others?
> 
> In my particular case, I was glancing through the logs, and I wanted to grab
> the second branch that someone else had made that was merged into pu. I would
> have loved to be able to run something like
> 
>   git merge origin/pu^{:nth(2)/nd/}
> 
> While we're speaking of modifiers, could we use one that says "only search
> the first parent hierarchy", e.g. something equivalent to git log's --first-parent
> flag?

As neat as this modifier syntax is getting, are we perhaps just
recreating the wheel?

How about:

  git merge `git rev-list -2 --grep=nd/ origin/pu | tail -1`

for the nth one, and:

  git merge `git rev-list --first-parent -1 --grep=nd/ origin/pu`

for a first parent search (I will leave combining them as an exercise to
the reader).

It's not that I'm opposed to a handy ref-specifying syntax. I just
wonder if it is really worth building in all of these obscure scenarios.

-Peff

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

> On Dec 10, 2010, at 11:03 AM, Jonathan Nieder wrote:
>
>> - What is the intended use for this family of modifiers?  I sort
>>   of understand ^{:i/... } for people that forget what case they
>>   have used, but why the :nth and others?
>
> In my particular case, I was glancing through the logs, and I wanted to grab
> the second branch that someone else had made that was merged into pu. I would
> have loved to be able to run something like
>
>   git merge origin/pu^{:nth(2)/nd/}
>
> While we're speaking of modifiers, could we use one that says "only search
> the first parent hierarchy", e.g. something equivalent to git log's --first-parent
> flag?

Both feels like a very made-up example to me.

The reason you can so sure that you can to give nth(2) not nth(3) nor
nth(1) and run "merge" in the example is probably because you looked at
the output from "git log --first-parent --oneline origin..origin/pu", no?

  d414638 Merge branch 'rj/msvc-fix' into pu
  e5f5e49 Merge branch 'ak/describe-exact' into pu
  439932d Merge branch 'jn/svn-fe' into pu
  d60b33b Merge branch 'pd/bash-4-completion' into pu
  09cbbde Merge branch 'tf/commit-list-prefix' into pu
  1b2ea00 Merge branch 'mg/cvsimport' into pu
  c6d41f4 Merge branch 'nd/maint-relative' into pu
  81f395e Merge branch 'ab/i18n' into pu
  9f5471f Merge branch 'nd/setup' into pu
  06f74a4 Merge branch 'yd/dir-rename' into pu
  d8a2ec8 Merge branch 'en/object-list-with-pathspec' into pu

After looking at this output, do you really want to say ":nth(2)/nd/"
instead of 9f5471f?

To come up with the "(2)" part you need to carefully scan the other lines
and make sure that there is only one "nd/" after what you want, and the
string does not appear in an unexpected places.

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Dec 10, 2010, at 3:08 PM, Junio C Hamano wrote:

> Kevin Ballard <kevin@sb.org> writes:
> 
>> On Dec 10, 2010, at 11:03 AM, Jonathan Nieder wrote:
>> 
>>> - What is the intended use for this family of modifiers?  I sort
>>>  of understand ^{:i/... } for people that forget what case they
>>>  have used, but why the :nth and others?
>> 
>> In my particular case, I was glancing through the logs, and I wanted to grab
>> the second branch that someone else had made that was merged into pu. I would
>> have loved to be able to run something like
>> 
>>  git merge origin/pu^{:nth(2)/nd/}
>> 
>> While we're speaking of modifiers, could we use one that says "only search
>> the first parent hierarchy", e.g. something equivalent to git log's --first-parent
>> flag?
> 
> Both feels like a very made-up example to me.

And yet the example I gave is pretty much precisely what prompted me to start
this discussion in the first place.

> The reason you can so sure that you can to give nth(2) not nth(3) nor
> nth(1) and run "merge" in the example is probably because you looked at
> the output from "git log --first-parent --oneline origin..origin/pu", no?
> 
>  d414638 Merge branch 'rj/msvc-fix' into pu
>  e5f5e49 Merge branch 'ak/describe-exact' into pu
>  439932d Merge branch 'jn/svn-fe' into pu
>  d60b33b Merge branch 'pd/bash-4-completion' into pu
>  09cbbde Merge branch 'tf/commit-list-prefix' into pu
>  1b2ea00 Merge branch 'mg/cvsimport' into pu
>  c6d41f4 Merge branch 'nd/maint-relative' into pu
>  81f395e Merge branch 'ab/i18n' into pu
>  9f5471f Merge branch 'nd/setup' into pu
>  06f74a4 Merge branch 'yd/dir-rename' into pu
>  d8a2ec8 Merge branch 'en/object-list-with-pathspec' into pu
> 
> After looking at this output, do you really want to say ":nth(2)/nd/"
> instead of 9f5471f?

Yep. Doing the latter either requires me to swap over to my mouse, copy the sha1,
and paste in, or requires me to peer at the sha1 and re-type enough characters.
It's a lot easier to just glance at that list, realize the 2nd one is the one I
want, and type `git merge :^{nth(2)/nd/}`. It may not necessarily be faster than
retyping the sha1, but it's a lot less prone to transcription errors.

-Kevin Ballard

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

> On Dec 10, 2010, at 3:08 PM, Junio C Hamano wrote:
> ...
>>  d414638 Merge branch 'rj/msvc-fix' into pu
>>  ...
>>  1b2ea00 Merge branch 'mg/cvsimport' into pu
>>  c6d41f4 Merge branch 'nd/maint-relative' into pu
>>  81f395e Merge branch 'ab/i18n' into pu
>>  9f5471f Merge branch 'nd/setup' into pu
>>  06f74a4 Merge branch 'yd/dir-rename' into pu
>>  d8a2ec8 Merge branch 'en/object-list-with-pathspec' into pu
>> 
>> After looking at this output, do you really want to say ":nth(2)/nd/"
>> instead of 9f5471f?
>
> Yep. Doing the latter either requires me to swap over to my mouse, copy the sha1,
> and paste in, or requires me to peer at the sha1 and re-type enough characters.
> It's a lot easier to just glance at that list, realize the 2nd one is the one I
> want, and type `git merge :^{nth(2)/nd/}`. It may not necessarily be faster than
> retyping the sha1, but it's a lot less prone to transcription errors.

What you said heavily depends on the way in which I give names to the
branches, and also on the fact that "nd/" happens to be not very common
prefix at this moment.  If the branches were named without nd/ part and
still be unique, you would not be arguing for nth(2) at all to begin with.

So it is dubious that your argument is convincing.

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Second, consider ref^{:nth(10)/foo} in your workaround...

Feels way over-engineered to me.

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Wed, 8 Dec 2010, Nguyễn Thái Ngọc Duy wrote:

> Let's start off from where the previous discussion [1] stopped. People
> seem to agree ref^{/regex} is a good choice. But we have not come to
> conclusion how to specify the count yet. Possible suggestions are
> 
>  - ref^{/foo}2
>  - ref^{2/foo}
>  - ref^{:2/foo}
>  - ref^{2nd/foo}
> 
> For whatever syntax chosen, :/ should benefit too. I notice that :/!
> is reserved for future use. Perhaps :/!2/regex is not too cryptic?

I wonder if it would be possible to make :/<regex> (which looks a bit
like searching the index) to be an alias to --all^{/<regex>}...

Or if we can make ^{/<regex>} to act on revision range specified by
earlier commits, so for example foo..bar^{/<regex>} would work.

> I'd also like to do case-insensitive regex, by the way. :/!2i/regex
> looks a bit ugly.

The '2nd' idea came from Perl 6 regexp / grammars, see for example
https://github.com/perlpilot/perl6-docs/blob/master/intro/p6-regex-intro.pod

 There are two other modifiers for matching a pattern some number of times
 or only matching, say, the third time we see a pattern in a string. These
 modifiers are a little strange in that their short-hand forms consist of
 a number followed by some text:

    modifier        short-hand              meaning
    :x()            :1x,:4x,:12x            match some number of times
    :nth()          :1st,:2nd,:3rd,:4th     match only the Nth occurance

 Here are some examples to illustrate these modifiers:

    $_ = "foo bar baz blat";
    m :3x/ a /              # matches the "a" characters in each word
    m :nth(3)/ \w+ /        # matches "baz"

So it could be e.g. 'foo^{:2nd/<regexp>}' (note that there is no trailing
/ closing regexp, i.e. it is not 'foo^{:2nd/<regexp>/}').

So if we chose this, why don't we follow Perl 6 rule of combining modifiers
http://perlcabal.org/syn/S05.html#Modifiers, so it would be

   foo^{:2nd:i/<regexp>}

or

   foo^{:i:nth(2)/<regexp>}


As to :/!<regexp> form: isn't it reserved for non-match?  If not, then
perhaps

  :/!2nd:i/<regexp>

> [1] http://mid.gmane.org/9D675671-693D-4B59-AF2A-0EFE4C537362@sb.org
> 
> Nguyễn Thái Ngọc Duy (2):
>   get_sha1_oneline: allow to input commit_list
>   get_sha1: support ref^{/regex} syntax
> 
>  Documentation/revisions.txt |    7 ++++++
>  sha1_name.c                 |   45 ++++++++++++++++++++++++++++++++----------
>  2 files changed, 41 insertions(+), 11 deletions(-)

Thank you for working on this.
-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jonathan Nieder]

Jakub Narebski wrote:

> So if we chose this, why don't we follow Perl 6 rule of combining modifiers
> http://perlcabal.org/syn/S05.html#Modifiers, so it would be
>
>    foo^{:2nd:i/<regexp>}
>
> or
>
>    foo^{:i:nth(2)/<regexp>}

Very nice.

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyen Thai Ngoc Duy]

2010/12/9 Jakub Narebski <jnareb@gmail.com>:
> I wonder if it would be possible to make :/<regex> (which looks a bit
> like searching the index) to be an alias to --all^{/<regex>}...

It looks a bit strange to my eyes to merge normal option name with
revision syntax. But I think it's possible. Do we allow branch/tag
name with leading '-'?

> Or if we can make ^{/<regex>} to act on revision range specified by
> earlier commits, so for example foo..bar^{/<regex>} would work.

There is another case: branch/tag selection. Instead of looking in all
refs, people may want to look only in nd/* branches. My branches are
almost flat, so I don't find any use. But someone might. And we can
solve the "all branches" case above with simply "*". The exact syntax,
I don't know.

> As to :/!<regexp> form: isn't it reserved for non-match?

It is reserved and not attached with any meaning.

> Thank you for working on this.

You're welcome. I needed to look for my branches in pu and was tired
of copy/paste.
-- 
Duy

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:

> > I wonder if it would be possible to make :/<regex> (which looks a bit
> > like searching the index) to be an alias to --all^{/<regex>}...
> 
> It looks a bit strange to my eyes to merge normal option name with
> revision syntax. But I think it's possible. Do we allow branch/tag
> name with leading '-'?

Well, with below proposal it would simply be

  --all ^{/<regexp>}
 
> > Or if we can make ^{/<regex>} to act on revision range specified by
> > earlier commits, so for example foo..bar^{/<regex>} would work.
> 
> There is another case: branch/tag selection. Instead of looking in all
> refs, people may want to look only in nd/* branches. My branches are
> almost flat, so I don't find any use. But someone might. And we can
> solve the "all branches" case above with simply "*". The exact syntax,
> I don't know.

  --glob=heads/nd/ ^{/<regexp>}

Similarly to the --all case.

-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyen Thai Ngoc Duy]

On Thu, Dec 9, 2010 at 7:44 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:
>
>> > I wonder if it would be possible to make :/<regex> (which looks a bit
>> > like searching the index) to be an alias to --all^{/<regex>}...
>>
>> It looks a bit strange to my eyes to merge normal option name with
>> revision syntax. But I think it's possible. Do we allow branch/tag
>> name with leading '-'?
>
> Well, with below proposal it would simply be
>
>  --all ^{/<regexp>}

This hardly works with range and may conflict with "--all" being
already used by some commands.

I think we can move '/' out of {}, the space between '/' and '{' can
be used for optional parameters: ^/{foo}.
-- 
Duy

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Dec 8, 2010, at 5:42 PM, Nguyen Thai Ngoc Duy wrote:

> On Thu, Dec 9, 2010 at 7:44 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>>> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:
>> 
>>>> I wonder if it would be possible to make :/<regex> (which looks a bit
>>>> like searching the index) to be an alias to --all^{/<regex>}...
>>> 
>>> It looks a bit strange to my eyes to merge normal option name with
>>> revision syntax. But I think it's possible. Do we allow branch/tag
>>> name with leading '-'?
>> 
>> Well, with below proposal it would simply be
>> 
>>  --all ^{/<regexp>}
> 
> This hardly works with range and may conflict with "--all" being
> already used by some commands.
> 
> I think we can move '/' out of {}, the space between '/' and '{' can
> be used for optional parameters: ^/{foo}

I thought ^{} was going to be an arbitrary grouping operator, capable of
embedding any other modifier, but primarily only useful for regex. This
change explicitly makes it an alternative regex syntax.

-Kevin Ballard

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
> On Thu, Dec 9, 2010 at 7:44 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>>> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:
>>
>>>> I wonder if it would be possible to make :/<regex> (which looks a bit
>>>> like searching the index) to be an alias to --all^{/<regex>}...
>>>
>>> It looks a bit strange to my eyes to merge normal option name with
>>> revision syntax. But I think it's possible. Do we allow branch/tag
>>> name with leading '-'?
>>
>> Well, with below proposal it would simply be
>>
>>  --all ^{/<regexp>}
> 
> This hardly works with range and may conflict with "--all" being
> already used by some commands.

It is '--all' like in "git log --all".
 
The proposed semantics for ^{/foo} (i.e. not attached to revision)
would be that it acts on all positive revs on the left of it, replacing
them.  But that might be not easy to do, and it feels a bit 
overengineered.

> I think we can move '/' out of {}, the space between '/' and '{' can
> be used for optional parameters: ^/{foo}.

Do you mean using e.g. ^/:i{foo} for :ignorecase, instead of ^{:i/foo}
or ^{i/foo}?

-- 
Jakub Narebski
Poland

Re: [PATCH 0/2] [RFD] Using gitrevisions :/search style with other operators

[Nguyen Thai Ngoc Duy]

On Thu, Dec 9, 2010 at 6:43 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>> On Thu, Dec 9, 2010 at 7:44 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> On Thu, 9 Dec 2010, Nguyen Thai Ngoc Duy wrote:
>>>> 2010/12/9 Jakub Narebski <jnareb@gmail.com>:
>>>
>>>>> I wonder if it would be possible to make :/<regex> (which looks a bit
>>>>> like searching the index) to be an alias to --all^{/<regex>}...
>>>>
>>>> It looks a bit strange to my eyes to merge normal option name with
>>>> revision syntax. But I think it's possible. Do we allow branch/tag
>>>> name with leading '-'?
>>>
>>> Well, with below proposal it would simply be
>>>
>>>  --all ^{/<regexp>}
>>
>> This hardly works with range and may conflict with "--all" being
>> already used by some commands.
>
> It is '--all' like in "git log --all".
>
> The proposed semantics for ^{/foo} (i.e. not attached to revision)
> would be that it acts on all positive revs on the left of it, replacing
> them.  But that might be not easy to do, and it feels a bit
> overengineered.

Yes, maybe.

>> I think we can move '/' out of {}, the space between '/' and '{' can
>> be used for optional parameters: ^/{foo}.
>
> Do you mean using e.g. ^/:i{foo} for :ignorecase, instead of ^{:i/foo}
> or ^{i/foo}?

Yes. I find "^/" easier to read than "^{/". But "^{/" is more
consistent to the rest.
-- 
Duy