Re: [RFC] Using gitrevisions :/search style with other operators

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

Jeff wrote:
> It seems to me the natural way to do that would be to use our existing
> generic "start at this ref and follow some chain" syntax, which is
> ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.

We may want to keep the "/" mnemonic (which seems no to conflict
withcurrent use either), rather than the ":" part, with something like
origin/pu^{/Merge 'kb/blame-author-email'}, and keep ":" for future use.

> We also have ref@{upstream}. The analogue here would be
> origin/pu@{:Merge 'kb/blame-author-email'}.

That's somewhat different, it looks like the foo@{...} only applies to
references with name "foo", and not to arbitrary revisions.  Allowing a
search to start from any commit seems more useful here.

Kevin wrote:
> Junio wrote:
> >    $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
[...]
>
> Interesting idea. It certainly solves the problem of being able to
> embed it within other operations (though you do then have to worry
> about escaping any embedded close-parens in the search), though it
> does mean my suggestion for being able to select the 2nd (or nth)
> match won't work.

Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
somewhat consistent with the commit^2 case, and would seem unambiguous
as well - a bit weird, though.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 8, 2010, at 11:30 PM, Yann Dirson wrote:

> Kevin wrote:
>> Junio wrote:
>>>   $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
> [...]
>> 
>> Interesting idea. It certainly solves the problem of being able to
>> embed it within other operations (though you do then have to worry
>> about escaping any embedded close-parens in the search), though it
>> does mean my suggestion for being able to select the 2nd (or nth)
>> match won't work.
> 
> Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> somewhat consistent with the commit^2 case, and would seem unambiguous
> as well - a bit weird, though.

This violates the idea that once you reach the end of a ^{} structure,
it resolves to a commit that can then be modified by subsequent operations.

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

On Tue, 09 Nov 2010 00:06:47 -0800
Kevin Ballard <kevin@sb.org> wrote:

> On Nov 8, 2010, at 11:30 PM, Yann Dirson wrote:
> 
> > Kevin wrote:
> >> Junio wrote:
> >>>   $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
> > [...]
> >> 
> >> Interesting idea. It certainly solves the problem of being able to
> >> embed it within other operations (though you do then have to worry
> >> about escaping any embedded close-parens in the search), though it
> >> does mean my suggestion for being able to select the 2nd (or nth)
> >> match won't work.
> > 
> > Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> > somewhat consistent with the commit^2 case, and would seem
> > unambiguous as well - a bit weird, though.
> 
> This violates the idea that once you reach the end of a ^{} structure,
> it resolves to a commit that can then be modified by subsequent
> operations.

OK, that's kinda related to the "looks weird" issue. 

Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}
Since the foo^{objecttype} syntax would not care for a count, it is not
a problem, and it keeps provision for using a count with future
operators.  OTOH, it could be a problem if we extend foo^{bar} to
accept "bar" for other things than object types.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Jeff King]

On Tue, Nov 09, 2010 at 08:30:23AM +0100, Yann Dirson wrote:

> Jeff wrote:
> > It seems to me the natural way to do that would be to use our existing
> > generic "start at this ref and follow some chain" syntax, which is
> > ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.
> 
> We may want to keep the "/" mnemonic (which seems no to conflict
> withcurrent use either), rather than the ":" part, with something like
> origin/pu^{/Merge 'kb/blame-author-email'}, and keep ":" for future use.

Yeah, sorry, the ':' thing was just a think-o on my part. It should
definitely be "/".

> > We also have ref@{upstream}. The analogue here would be
> > origin/pu@{:Merge 'kb/blame-author-email'}.
> 
> That's somewhat different, it looks like the foo@{...} only applies to
> references with name "foo", and not to arbitrary revisions.  Allowing a
> search to start from any commit seems more useful here.

Yeah, agreed.

-Peff

Re: [RFC] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Yann Dirson <dirson@bertin.fr> writes:

>> > Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> ...
> Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}

What are these "2"s?

You need to question how you figured out the commit you want is the second
one reachable (in whatever traversal order) from something in the first
place.  Didn't you use "git log --oneline" or something to find that out?
At that point, you have the object name already, so I doubt such a
"counting" feature is of much practical use.

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 9, 2010, at 4:18 PM, Junio C Hamano wrote:

> Yann Dirson <dirson@bertin.fr> writes:
> 
>>>> Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
>> ...
>> Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}
> 
> What are these "2"s?
> 
> You need to question how you figured out the commit you want is the second
> one reachable (in whatever traversal order) from something in the first
> place.  Didn't you use "git log --oneline" or something to find that out?
> At that point, you have the object name already, so I doubt such a
> "counting" feature is of much practical use.

The particular case that prompted this for me was I knew I had created two
commits called "WIP", scheduled for renaming later, and I wanted to quickly
look at the contents of the first one. I would have loved to be able to
type something like `git show :/WIP/2`. I suppose this situation may be rare
enough not to bother supporting it in the new syntax. With the new syntax
it will be possible to do something like `git show HEAD^{:/WIP}^^{:/WIP}`, but
that looks awfully awkward.

Another thing to consider - the current :/foo syntax searches for the newest
commit reachable from any ref. Using the ^{} syntax will require specifying
a ref first. I'm not sure this is a problem though, as I'm not really sure why
:/foo searches from all refs to begin with.

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

On Tue, 09 Nov 2010 16:33:31 -0800
Kevin Ballard <kevin@sb.org> wrote:

> On Nov 9, 2010, at 4:18 PM, Junio C Hamano wrote:
> 
> > Yann Dirson <dirson@bertin.fr> writes:
> > 
> >>>> Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> >> ...
> >> Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}
> > 
> > What are these "2"s?
> > 
> > You need to question how you figured out the commit you want is the
> > second one reachable (in whatever traversal order) from something
> > in the first place.  Didn't you use "git log --oneline" or
> > something to find that out? At that point, you have the object name
> > already, so I doubt such a "counting" feature is of much practical
> > use.

I usually always have a gitk displaying history.  Whereas it shows the
commit summary, it does not show the sha1 for each commit - and even if
it did, my brain is still more comfortable dealing with words than
hashes (though that may arguably be an effect of aging ;)

> The particular case that prompted this for me was I knew I had
> created two commits called "WIP", scheduled for renaming later, and I
> wanted to quickly look at the contents of the first one. I would have
> loved to be able to type something like `git show :/WIP/2`. I suppose
> this situation may be rare enough not to bother supporting it in the
> new syntax. With the new syntax it will be possible to do something
> like `git show HEAD^{:/WIP}^^{:/WIP}`, but that looks awfully awkward.

Another use for counting would be for reflog, to lookup things like
"2nd to last of yesterday's commits" - that could be spelled like
"master^{:2:yesterday}" or similar.  Not sure it's worth it (I hardly
use the @{anything vague} syntax myself, especially because it is so
vague), but that looked similar enough to be mentionned here.

> Another thing to consider - the current :/foo syntax searches for the
> newest commit reachable from any ref. Using the ^{} syntax will
> require specifying a ref first. I'm not sure this is a problem
> though, as I'm not really sure why :/foo searches from all refs to
> begin with.

The syntax could be extended so that ^{whatever} starts looking at
current commit (ie. HEAD), somewhat like @{whatever} looks at reflog for
current branch.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

On Tue, 09 Nov 2010 23:46:59 -0800
Kevin Ballard <kevin@sb.org> wrote:

> On Nov 9, 2010, at 11:32 PM, Yann Dirson wrote:
> 
> >> Another thing to consider - the current :/foo syntax searches for
> >> the newest commit reachable from any ref. Using the ^{} syntax will
> >> require specifying a ref first. I'm not sure this is a problem
> >> though, as I'm not really sure why :/foo searches from all refs to
> >> begin with.
> > 
> > The syntax could be extended so that ^{whatever} starts looking at
> > current commit (ie. HEAD), somewhat like @{whatever} looks at
> > reflog for current branch.
> 
> :/foo doesn't start from the current commit - it searches all refs.
> However, making ^{} search all refs if not given one doesn't make
> sense for any operator except :/foo, so I don't think it's worth doing

Yes, that's why I suggested to make it search from HEAD, not from all
refs.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 9, 2010, at 11:32 PM, Yann Dirson wrote:

>> Another thing to consider - the current :/foo syntax searches for the
>> newest commit reachable from any ref. Using the ^{} syntax will
>> require specifying a ref first. I'm not sure this is a problem
>> though, as I'm not really sure why :/foo searches from all refs to
>> begin with.
> 
> The syntax could be extended so that ^{whatever} starts looking at
> current commit (ie. HEAD), somewhat like @{whatever} looks at reflog for
> current branch.

:/foo doesn't start from the current commit - it searches all refs. However,
making ^{} search all refs if not given one doesn't make sense for any
operator except :/foo, so I don't think it's worth doing

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Jakub Narebski]

Yann Dirson <dirson@bertin.fr> writes:
> On Tue, 09 Nov 2010 23:46:59 -0800
> Kevin Ballard <kevin@sb.org> wrote:
>> On Nov 9, 2010, at 11:32 PM, Yann Dirson wrote:
>> 
>>>> Another thing to consider - the current :/foo syntax searches for
>>>> the newest commit reachable from any ref. Using the ^{} syntax will
>>>> require specifying a ref first. I'm not sure this is a problem
>>>> though, as I'm not really sure why :/foo searches from all refs to
>>>> begin with.
>>> 
>>> The syntax could be extended so that ^{whatever} starts looking at
>>> current commit (ie. HEAD), somewhat like @{whatever} looks at
>>> reflog for current branch.

The <ref>@<sth> is about reflogs: <ref>@{<n>}, <ref>@{<aproxidate>},
@{-<n>}, <ref>@{upstream} / <ref>@{u}.  Because HEAD has separate
reflog, then @{<sth>} is about current branch reflog (@{-<n>} uses
HEAD reflog, though).  <ref> must be something that has reflog.

The <obj>^<sth> is about dereferencing: <tag>^{<objtype>} and <tag>^{},
which includes following parents <commit>^ and <commit>^<n>.  Then
there are odd <commit-ish>^! (returning range) and <commit-ish>^@
(all parents) but which nevertheless follow this rule.

The <obj>:<sth> is (with single exception of ':/<regexp>') about
selecting subitem (path): <tree-ish>:<path>, [:<stage>]:<path>

>> 
>> :/foo doesn't start from the current commit - it searches all refs.
>> However, making ^{} search all refs if not given one doesn't make
>> sense for any operator except :/foo, so I don't think it's worth doing
> 
> Yes, that's why I suggested to make it search from HEAD, not from all
> refs.

Perhaps '--all^{/foo}'?  Just kidding... I think ;-)


About n-th match: I think that ^{<n>/foo} or ^{:<n>/foo}... or
^{:nth(<n>)/foo} as generic form of e.g. ^{3rd/foo} a la Perl 6 :-)
Or perhaps even full form ^{m:nth(<n>)/foo}... well, perhaps not.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

[PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

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

Currently :path and ref:path can be used to refer to a specific object
in index or ref respectively. "path" component is absolute path. This
patch allows "path" to be written as "./path" or "../path", which is
relative to user's original cwd.

This does not work in commands that startup_info is NULL
(i.e. non-builtin ones).
---
 On Wed, Nov 10, 2010 at 07:26:20AM -0800, Jakub Narebski wrote:
 > The <obj>:<sth> is (with single exception of ':/<regexp>') about
 > selecting subitem (path): <tree-ish>:<path>, [:<stage>]:<path>

 I feel the urge of keeping ':./path' and ':../path' out of this
 competition.

 The idea is old although I don't remember if anybody has made any
 attempt to realize it: use './' and '../' to specify the given path
 is relative, not absolute.

 I don't remember either if the idea was rejected or nobody bothered
 to implement it. Anyway, here it is (for demostration only because it
 needs two minor patches to work). Comments?
 --
 Duy

 sha1_name.c |   30 +++++++++++++++++++++++++++---
 1 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 484081d..791608d 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1060,25 +1060,35 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 	if (!ret)
 		return ret;
 	/* sha1:path --> object name of path in ent sha1
-	 * :path -> object name of path in index
+	 * :path -> object name of absolute path in index
+	 * :./path -> object name of path relative to cwd in index
 	 * :[0-3]:path -> object name of path in index at stage
 	 * :/foo -> recent commit matching foo
 	 */
 	if (name[0] == ':') {
 		int stage = 0;
 		struct cache_entry *ce;
+		char *new_path = NULL;
 		int pos;
 		if (namelen > 2 && name[1] == '/')
 			return get_sha1_oneline(name + 2, sha1);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
-		    name[1] < '0' || '3' < name[1])
+		    name[1] < '0' || '3' < name[1]) {
 			cp = name + 1;
+			if (startup_info && cp[0] == '.' &&
+			    (cp[1] == '/' || (cp[1] == '.' && cp[2] == '/'))) {
+				new_path = prefix_path(startup_info->prefix,
+						       strlen(startup_info->prefix),
+						       cp);
+				cp = new_path;
+			}
+		}
 		else {
 			stage = name[1] - '0';
 			cp = name + 3;
 		}
-		namelen = namelen - (cp - name);
+		namelen = strlen(cp);
 
 		strncpy(oc->path, cp,
 			sizeof(oc->path));
@@ -1096,12 +1106,14 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				break;
 			if (ce_stage(ce) == stage) {
 				hashcpy(sha1, ce->sha1);
+				free(new_path);
 				return 0;
 			}
 			pos++;
 		}
 		if (!gently)
 			diagnose_invalid_index_path(stage, prefix, cp);
+		free(new_path);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -1122,6 +1134,17 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 		}
 		if (!get_sha1_1(name, cp-name, tree_sha1)) {
 			const char *filename = cp+1;
+			char *new_filename = NULL;
+
+			if (startup_info &&
+			    filename[0] == '.' &&
+			    (filename[1] == '/' ||
+			     (filename[1] == '.' && filename[2] == '/'))) {
+				new_filename = prefix_path(startup_info->prefix,
+							   strlen(startup_info->prefix),
+							   filename);
+				filename = new_filename;
+			}
 			ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
 			if (!gently) {
 				diagnose_invalid_sha1_path(prefix, filename,
@@ -1133,6 +1156,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 				sizeof(oc->path));
 			oc->path[sizeof(oc->path)-1] = '\0';
 
+			free(new_filename);
 			return ret;
 		} else {
 			if (!gently)
-- 
1.7.3.2.210.g045198

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Matthieu Moy]

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

> Currently :path and ref:path can be used to refer to a specific object
> in index or ref respectively. "path" component is absolute path. This
> patch allows "path" to be written as "./path" or "../path", which is
> relative to user's original cwd.
>
> This does not work in commands that startup_info is NULL

Grammar: shouldn't it be "commands for which startup_info is NULL"?

> (i.e. non-builtin ones).

Does that include "git show"? That's probably the most common use-case
for the <object>:<path> syntax, perhaps worth mentionning here.

Also, what happens when it doesn't work? I guess it falls back to the
previous behavior, which tries to do a detailed diagnosis and provides
a general error message. But probably the error messages should be
reworded to include "syntax not supported by this command" or so,
'cause it's really weird for a user to be able to say HEAD:./foo in
some places and not in others.

>  The idea is old although I don't remember if anybody has made any
>  attempt to realize it: use './' and '../' to specify the given path
>  is relative, not absolute.

I gave it a try, but I was not aware of the startup_info thing, and
tried passing more arguments to get_sha1*, and you can guess I quickly
gave up ;-).

> +			if (startup_info && cp[0] == '.' &&
> +			    (cp[1] == '/' || (cp[1] == '.' && cp[2] == '/'))) {

Nothing performance-critical here, so I'd rather have something more
readable, like

startup_info && (!prefixcmp(cp, "./") || !prefixcmp(cp, "../"))


> +				new_path = prefix_path(startup_info->prefix,
> +						       strlen(startup_info->prefix),
> +						       cp);

free(cp); here?

> +				cp = new_path;
> +			}
> +		}

> @@ -1122,6 +1134,17 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
>  		}
>  		if (!get_sha1_1(name, cp-name, tree_sha1)) {
>  			const char *filename = cp+1;
> +			char *new_filename = NULL;
> +
> +			if (startup_info &&
> +			    filename[0] == '.' &&
> +			    (filename[1] == '/' ||
> +			     (filename[1] == '.' && filename[2] == '/'))) {
> +				new_filename = prefix_path(startup_info->prefix,
> +							   strlen(startup_info->prefix),
> +							   filename);

Same 2 questions as above.

Also, that would be nice is this came with a testcase :-)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> The <ref>@<sth> is about reflogs

Wrong.  "@" in the olden days were limited about reflogs but not these
days.  <ref>@<something> is about refs; <objectname>^<operation> is about
objects (most often commits).

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Jonathan Nieder]

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

> This does not work in commands that startup_info is NULL
> (i.e. non-builtin ones).

That is easily fixable, no?

The non-builtins are:

	fast-import
	imap-send
	shell
	daemon
	show-index
	upload-pack
	http-backend
	http-fetch
	http-push

Of those, only fast-import might have a possible reason need to know
about this new syntax.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Assuming your patch builds on acf4823a (setup: save prefix (original
 cwd relative to toplevel) in startup_info, 2010-10-24).

 Untested.

diff --git a/fast-import.c b/fast-import.c
index 77549eb..3b597e8 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2923,8 +2923,11 @@ static void parse_argv(void)
 
 int main(int argc, const char **argv)
 {
+	static struct startup_info git_startup_info;
 	unsigned int i;
 
+	startup_info = &git_startup_info;
+
 	git_extract_argv0_path(argv[0]);
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))

Re: [RFC] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Wed, 10 Nov 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > The <ref>@<sth> is about reflogs
> 
> Wrong.  "@" in the olden days were limited about reflogs but not these
> days.  <ref>@<something> is about refs; <objectname>^<operation> is about
> objects (most often commits).

Fact.  I even provided example, and didn't notice it: [<ref>]@{upstream}
does not use reflogs, but config.
-- 
Jakub Narebski
Poland

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Junio C Hamano]

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

>  		if (namelen < 3 ||
>  		    name[2] != ':' ||
> -		    name[1] < '0' || '3' < name[1])
> +		    name[1] < '0' || '3' < name[1]) {
>  			cp = name + 1;
> +			if (startup_info && cp[0] == '.' &&
> +			    (cp[1] == '/' || (cp[1] == '.' && cp[2] == '/'))) {
> +				new_path = prefix_path(startup_info->prefix,
> +						       strlen(startup_info->prefix),
> +						       cp);
> +				cp = new_path;
> +			}
> +		}

What does this new codepath do when we know where the working tree is and
we are outside of the working tree (e.g. we are looking at the index or a
commit in a submodule from above the superproject)?

What should it do?

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Nguyen Thai Ngoc Duy]

2010/11/11 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>>               if (namelen < 3 ||
>>                   name[2] != ':' ||
>> -                 name[1] < '0' || '3' < name[1])
>> +                 name[1] < '0' || '3' < name[1]) {
>>                       cp = name + 1;
>> +                     if (startup_info && cp[0] == '.' &&
>> +                         (cp[1] == '/' || (cp[1] == '.' && cp[2] == '/'))) {
>> +                             new_path = prefix_path(startup_info->prefix,
>> +                                                    strlen(startup_info->prefix),
>> +                                                    cp);
>> +                             cp = new_path;
>> +                     }
>> +             }
>
> What does this new codepath do when we know where the working tree is and
> we are outside of the working tree (e.g. we are looking at the index or a
> commit in a submodule from above the superproject)?
>
> What should it do?

Prefix is NULL in that case. So no prefixing, git will complain the
given path does not exist (although it should say "./" syntax is not
usable without a worktree).

prefix_path() dies if the resolved path is outside repo. So no, users
can't look up files in superproject from a subproject. I think we need
to define the superproject/subproject relation first. I have
$HOME/.git, but I don't think it's a superproject to any repos inside
my $HOME.
-- 
Duy

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Nguyen Thai Ngoc Duy]

2010/11/11 Jonathan Nieder <jrnieder@gmail.com>:
> Nguyễn Thái Ngọc Duy wrote:
>
>> This does not work in commands that startup_info is NULL
>> (i.e. non-builtin ones).
>
> That is easily fixable, no?

Laziness is a big factor to me (or maybe I'm just tired after work).
In the meantime I'll take your analysis and fix fast-import. Thanks.
-- 
Duy

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Junio C Hamano]

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

> Currently :path and ref:path can be used to refer to a specific object
> in index or ref respectively. "path" component is absolute path. This
> patch allows "path" to be written as "./path" or "../path", which is
> relative to user's original cwd.
>
> This does not work in commands that startup_info is NULL
> (i.e. non-builtin ones).
> ---
>  On Wed, Nov 10, 2010 at 07:26:20AM -0800, Jakub Narebski wrote:
>  > The <obj>:<sth> is (with single exception of ':/<regexp>') about
>  > selecting subitem (path): <tree-ish>:<path>, [:<stage>]:<path>
>
>  I feel the urge of keeping ':./path' and ':../path' out of this
>  competition.
>
>  The idea is old although I don't remember if anybody has made any
>  attempt to realize it: use './' and '../' to specify the given path
>  is relative, not absolute.

I've had this queued for some time, with help from Jonathan, and am happy
about it in general, but after taking another look at it, noticed one
minor thing.

> diff --git a/sha1_name.c b/sha1_name.c
> index 484081d..791608d 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1060,25 +1060,35 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
>  	if (!ret)
>  		return ret;
>  	/* sha1:path --> object name of path in ent sha1
> -	 * :path -> object name of path in index
> +	 * :path -> object name of absolute path in index
> +	 * :./path -> object name of path relative to cwd in index
>  	 * :[0-3]:path -> object name of path in index at stage
>  	 * :/foo -> recent commit matching foo
>  	 */
>  	if (name[0] == ':') {
>  		int stage = 0;
>  		struct cache_entry *ce;
> +		char *new_path = NULL;
>  		int pos;
>  		if (namelen > 2 && name[1] == '/')
>  			return get_sha1_oneline(name + 2, sha1);
>  		if (namelen < 3 ||
>  		    name[2] != ':' ||
> -		    name[1] < '0' || '3' < name[1])
> +		    name[1] < '0' || '3' < name[1]) {
>  			cp = name + 1;
> +			if (startup_info && cp[0] == '.' &&
> +			    (cp[1] == '/' || (cp[1] == '.' && cp[2] == '/'))) {
> +				new_path = prefix_path(startup_info->prefix,
> +						       strlen(startup_info->prefix),
> +						       cp);
> +				cp = new_path;
> +			}
> +		}

This deals with ":$foo" (path $foo in the index), which is a shorthand for
":0:$foo" (the path $foo at stage #0 in the index).  However...

>  		else {
>  			stage = name[1] - '0';
>  			cp = name + 3;

... this side does not do anything about it, so:

	$ cd Documentation
        ... working for a while ...
        $ git merge another_branch
        ... oops, got conflicts at ../Makefile
	$ git show :2:../Makefile

won't work.

Besides, it is a bad idea to make ":../Makefile" work while leaving
its longhand ":0:../Makefile" not.

I think it is just the matter of moving "if (startup-info)..." logic
outside the "is the :$path lacking an explicit stage number" block and
having it after that if/else, no?

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Junio C Hamano]

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

> I think it is just the matter of moving "if (startup-info)..." logic
> outside the "is the :$path lacking an explicit stage number" block and
> having it after that if/else, no?

Like this, perhaps?

-- >8 --
Subject: get_sha1: teach ":$n:<path>" the same relative path logic

Earlier we taught the object name parser ":<path>" syntax that uses a path
relative to the current working directory.  Given that ":<path>" is just a
short-hand for ":0:<path>" (i.e. "take stage #0 of that path"), we should
allow ":$n:<path>" to use relative path the same way.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_name.c                    |   14 ++++++++------
 t/t1506-rev-parse-diagnosis.sh |   24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index f918faf..2074056 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1091,17 +1091,19 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
 			return get_sha1_oneline(name + 2, sha1);
 		if (namelen < 3 ||
 		    name[2] != ':' ||
-		    name[1] < '0' || '3' < name[1]) {
+		    name[1] < '0' || '3' < name[1])
 			cp = name + 1;
-			new_path = resolve_relative_path(cp);
-			if (new_path)
-				cp = new_path;
-		}
 		else {
 			stage = name[1] - '0';
 			cp = name + 3;
 		}
-		namelen = strlen(cp);
+		new_path = resolve_relative_path(cp);
+		if (!new_path) {
+			namelen = namelen - (cp - name);
+		} else {
+			cp = new_path;
+			namelen = strlen(cp);
+		}
 
 		strncpy(oc->path, cp,
 			sizeof(oc->path));
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 1866470..9f8adb1 100755
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -34,6 +34,8 @@ test_expect_success 'correct file objects' '
 test_expect_success 'correct relative file objects (0)' '
 	git rev-parse :file.txt >expected &&
 	git rev-parse :./file.txt >result &&
+	test_cmp expected result &&
+	git rev-parse :0:./file.txt >result &&
 	test_cmp expected result
 '
 
@@ -68,6 +70,28 @@ test_expect_success 'correct relative file objects (4)' '
 	)
 '
 
+test_expect_success 'correct relative file objects (5)' '
+	git rev-parse :subdir/file.txt >expected &&
+	(
+		cd subdir &&
+		git rev-parse :./file.txt >result &&
+		test_cmp ../expected result &&
+		git rev-parse :0:./file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
+test_expect_success 'correct relative file objects (6)' '
+	git rev-parse :file.txt >expected &&
+	(
+		cd subdir &&
+		git rev-parse :../file.txt >result &&
+		test_cmp ../expected result &&
+		git rev-parse :0:../file.txt >result &&
+		test_cmp ../expected result
+	)
+'
+
 test_expect_success 'incorrect revision id' '
 	test_must_fail git rev-parse foobar:file.txt 2>error &&
 	grep "Invalid object name '"'"'foobar'"'"'." error &&

Re: [PATCH] get_sha1: support relative path "<obj>:<sth>" syntax

[Nguyen Thai Ngoc Duy]

2010/12/10 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I think it is just the matter of moving "if (startup-info)..." logic
>> outside the "is the :$path lacking an explicit stage number" block and
>> having it after that if/else, no?
>
> Like this, perhaps?

Yeah, looks good. Sorry I missed that else block.
-- 
Duy