Re: PATCH: allow ':/<oneline prefix>' notation to specify a specific file

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

 



"Eric Raible" <raible@xxxxxxxxx> writes:

> This patch allows git show ":/PATCH: allow":sha1_name.c to show the
> change to the file changed by this patch.
> ...
> @@ -697,8 +698,18 @@ int get_sha1_with_mode(const char *name, unsigned
> char *sha1, unsigned *mode)
>  		int stage = 0;
>  		struct cache_entry *ce;
>  		int pos;
> -		if (namelen > 2 && name[1] == '/')
> -			return get_sha1_oneline(name + 2, sha1);
> +		if (namelen > 2 && name[1] == '/') {
> +			name += 2;
> +			colon = strrchr(name, ':');
> +			if (!get_sha1_oneline(name, sha1) || !colon)
> +				return 0;

So when you have ":/A:B:C", you first try to look for string "A:B:C", and
then when it fails try "A:B" and look for path C?  I think this fallback
makes sense, especially because this cannot break existing use for
positive lookup (it _can_ be called a regression if you are checking to
see if you have a commit that has A:B:C and you want the lookup to fail if
there is A:B that happens to have path C, but I do not think we would care
about that usage).

A few observations:

 (1) An obvious micro-optimization. Check if "A:B:C" is a rev, and if and
     only if it fails, run strrchr() to find fallback point;

 (2) When you do not find either "A:B:C" nor "A:B", perhaps try "A" and
     find "B:C" as a path in it?  IOW, you may want to fallback more than
     once;

 (3) If you are given ":/A~20" or ":/A^2", perhaps you would want to do
     something similar?

Other than that, I like what the patch is trying to do.     
--
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]

  Powered by Linux