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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2010, 21:10 UTC
Message-ID: <7vipz2fy98.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1289407021-30287-1-git-send-email-pclouds@gmail.com>

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

Show 18 quoted lines

> 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.

Show 34 quoted lines

> 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?