"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