{"thread":{"id":"14272","subject":"Re: PATCH: allow ':/' notation to specify a specific file","startedAt":"2008-07-03T08:52:45Z","lastAt":"2008-07-03T12:55:50Z","messageCount":4,"participants":["Junio C Hamano","Eric Raible","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"82120","messageId":"279b37b20807030152g13492d5dxf21367ab17719993@mail.gmail.com","threadId":"14272","inReplyTo":null,"subject":"PATCH: allow ':/' notation to specify a specific file","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2008-07-03T08:52:45Z","receivedAt":"2008-07-03T08:52:45Z","isPatch":false,"sender":{"key":"raible@gmail.com","avatar":null},"body":"Although the rev-parse documentation claims that the tree-ish:path/to/file\nsyntax is applicable to all tree-ish forms this is not so when using the\n:/ \"oneline prefix\" syntax introduced in v1.5.0.1-227-g28a4d94.\n\nThis patch allows git show \":/PATCH: allow\":sha1_name.c to show the\nchange to the file changed by this patch.\n\nSigned-off-by: Eric Raible \n---\n sha1_name.c | 15 +++++++++++++--\n 1 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex b0b2167..a1acfcd 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -684,6 +684,7 @@ int get_sha1_with_mode(const char *name, unsigned\nchar *sha1, unsigned *mode)\n \tint ret, bracket_depth;\n \tint namelen = strlen(name);\n \tconst char *cp;\n+\tchar *copy, *colon;\n\n \t*mode = S_IFINVALID;\n \tret = get_sha1_1(name, namelen, sha1);\n@@ -697,8 +698,18 @@ int get_sha1_with_mode(const char *name, unsigned\nchar *sha1, unsigned *mode)\n \t\tint stage = 0;\n \t\tstruct cache_entry *ce;\n \t\tint pos;\n-\t\tif (namelen > 2 && name[1] == '/')\n-\t\t\treturn get_sha1_oneline(name + 2, sha1);\n+\t\tif (namelen > 2 && name[1] == '/') {\n+\t\t\tname += 2;\n+\t\t\tcolon = strrchr(name, ':');\n+\t\t\tif (!get_sha1_oneline(name, sha1) || !colon)\n+\t\t\t\treturn 0;\n+\t\t\tcopy = xstrdup(name);\n+\t\t\t*(colon = strrchr(copy, ':')) = '\\0';\n+\t\t\tret = get_sha1_oneline(copy, sha1) ||\n+\t\t\t\tget_tree_entry(sha1, colon+1, sha1, mode);\n+\t\t\tfree(copy);\n+\t\t\treturn ret;\n+\t\t}\n \t\tif (namelen < 3 ||\n \t\t name[2] != ':' ||\n \t\t name[1] < '0' || '3' < name[1])\n-- \n1.5.6.1.1356.g3be5f.dirty\n"},{"id":"82103","messageId":"7v7ic32ujy.fsf@gitster.siamese.dyndns.org","threadId":"14272","inReplyTo":"279b37b20807030152g13492d5dxf21367ab17719993@mail.gmail.com","subject":"Re: PATCH: allow ':/' notation to specify a specific file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-03T09:20:17Z","receivedAt":"2008-07-03T09:20:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric Raible\" writes:\n\n> This patch allows git show \":/PATCH: allow\":sha1_name.c to show the\n> change to the file changed by this patch.\n> ...\n> @@ -697,8 +698,18 @@ int get_sha1_with_mode(const char *name, unsigned\n> char *sha1, unsigned *mode)\n> \t\tint stage = 0;\n> \t\tstruct cache_entry *ce;\n> \t\tint pos;\n> -\t\tif (namelen > 2 && name[1] == '/')\n> -\t\t\treturn get_sha1_oneline(name + 2, sha1);\n> +\t\tif (namelen > 2 && name[1] == '/') {\n> +\t\t\tname += 2;\n> +\t\t\tcolon = strrchr(name, ':');\n> +\t\t\tif (!get_sha1_oneline(name, sha1) || !colon)\n> +\t\t\t\treturn 0;\n\nSo when you have \":/A:B:C\", you first try to look for string \"A:B:C\", and\nthen when it fails try \"A:B\" and look for path C? I think this fallback\nmakes sense, especially because this cannot break existing use for\npositive lookup (it _can_ be called a regression if you are checking to\nsee if you have a commit that has A:B:C and you want the lookup to fail if\nthere is A:B that happens to have path C, but I do not think we would care\nabout that usage).\n\nA few observations:\n\n (1) An obvious micro-optimization. Check if \"A:B:C\" is a rev, and if and\n only if it fails, run strrchr() to find fallback point;\n\n (2) When you do not find either \"A:B:C\" nor \"A:B\", perhaps try \"A\" and\n find \"B:C\" as a path in it? IOW, you may want to fallback more than\n once;\n\n (3) If you are given \":/A~20\" or \":/A^2\", perhaps you would want to do\n something similar?\n\nOther than that, I like what the patch is trying to do. \n"},{"id":"82122","messageId":"alpine.DEB.1.00.0807031352350.9925@racer","threadId":"14272","inReplyTo":"279b37b20807030152g13492d5dxf21367ab17719993@mail.gmail.com","subject":"Re: PATCH: allow ':/' notation to specify a specific file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-03T12:53:13Z","receivedAt":"2008-07-03T12:53:13Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nis there a reason you break the mail thread quite a couple of times? It \nmakes it really hard to follow when you get 300+ mails a day.\n\nThankyouverymuch,\nDscho\n"},{"id":"82125","messageId":"alpine.DEB.1.00.0807031353250.9925@racer","threadId":"14272","inReplyTo":"7v7ic32ujy.fsf@gitster.siamese.dyndns.org","subject":"Re: PATCH: allow ':/' notation to specify a specific file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-03T12:55:50Z","receivedAt":"2008-07-03T12:55:50Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 3 Jul 2008, Junio C Hamano wrote:\n\n> \"Eric Raible\" writes:\n> \n> > This patch allows git show \":/PATCH: allow\":sha1_name.c to show the\n> > change to the file changed by this patch.\n> > ...\n> > @@ -697,8 +698,18 @@ int get_sha1_with_mode(const char *name, unsigned\n> > char *sha1, unsigned *mode)\n> > \t\tint stage = 0;\n> > \t\tstruct cache_entry *ce;\n> > \t\tint pos;\n> > -\t\tif (namelen > 2 && name[1] == '/')\n> > -\t\t\treturn get_sha1_oneline(name + 2, sha1);\n> > +\t\tif (namelen > 2 && name[1] == '/') {\n> > +\t\t\tname += 2;\n> > +\t\t\tcolon = strrchr(name, ':');\n> > +\t\t\tif (!get_sha1_oneline(name, sha1) || !colon)\n> > +\t\t\t\treturn 0;\n> \n> So when you have \":/A:B:C\", you first try to look for string \"A:B:C\", and\n> then when it fails try \"A:B\" and look for path C? I think this fallback\n> makes sense, especially because this cannot break existing use for\n> positive lookup (it _can_ be called a regression if you are checking to\n> see if you have a commit that has A:B:C and you want the lookup to fail if\n> there is A:B that happens to have path C, but I do not think we would care\n> about that usage).\n\nHowever, if you specify ambiguous information, you can end up with a \ncommit when you expect a file.\n\nI do not like the direction this is going; in hindsight, I think \n\":/\" was a serious mistake.\n\nAs I hinted in another mail, which should have been in the same mail \nthread, \"log --grep\" is so much more powerful and should supersede \n\":/\".\n\nLet's grant \":/\" a quick and painless death.\n\nCiao,\nDscho\n"}]}