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

Yann Dirson [off-list ref] writes:

quoted

quoted

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.

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

Yann Dirson [off-list ref] writes:

quoted

quoted

quoted

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

On Tue, 09 Nov 2010 16:33:31 -0800
Kevin Ballard [off-list ref] wrote:

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

quoted

Yann Dirson [off-list ref] writes:

quoted

quoted

quoted

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

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

quoted

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

On Tue, 09 Nov 2010 23:46:59 -0800
Kevin Ballard [off-list ref] wrote:

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

quoted

quoted

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

Yann Dirson [off-list ref] writes:

On Tue, 09 Nov 2010 23:46:59 -0800
Kevin Ballard [off-list ref] wrote:

quoted

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

quoted

quoted

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>

quoted

:/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

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

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 <redacted>
---
 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"))

2010/11/11 Jonathan Nieder [off-list ref]:

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

quoted

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