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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyán ThÃi Ngác Duy  <pclouds@xxxxxxxxx> 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?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]