{"thread":{"id":"26939","subject":"Usability improvement request: git show revision -- file","startedAt":"2011-03-31T06:45:07Z","lastAt":"2011-04-05T06:06:51Z","messageCount":29,"participants":["Piotr Krukowiecki","Michael J Gruber","Johannes Sixt","Nguyen Thai Ngoc Duy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"164760","messageId":"AANLkTi=JPVXD7Je7Rz9yhKHnJo+K2rs5WnwTY22c3h6L@mail.gmail.com","threadId":"26939","inReplyTo":null,"subject":"Usability improvement request: git show revision -- file","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-31T06:45:07Z","receivedAt":"2011-03-31T06:45:07Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"Hi,\n\nif there's existing way to do this please tell me.\n\nThere's this file \"src/subdir/file\". I'm in the \"src\" directory and want to\nsee the \"file\" at specific revision.\n\nKnowing about git show I'd expect something like this to work:\n\n $ git show master -- subdir/file\n\nBut it shows nothing (no output, no warning). Following also does not\nwork as expected:\n\n $ git show master:subdir/file\n fatal: Path 'src/subdir/file' exists, but not 'subdir/file'.\n Did you mean 'master:src/subdir/file'?\n\nOf course following works:\n\n $ git show master:src/subdir/file\n\nbut it's not very convenient to have to specify full path, and it's not what\nyou would expect given that most other commands accept \"-- relativepath\"\nsyntax.\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"164762","messageId":"4D94322A.8030409@drmicha.warpmail.net","threadId":"26939","inReplyTo":"AANLkTi=JPVXD7Je7Rz9yhKHnJo+K2rs5WnwTY22c3h6L@mail.gmail.com","subject":"Re: Usability improvement request: git show revision -- file","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T07:50:02Z","receivedAt":"2011-03-31T07:50:02Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Piotr Krukowiecki venit, vidit, dixit 31.03.2011 08:45:\n> Hi,\n> \n> if there's existing way to do this please tell me.\n> \n> There's this file \"src/subdir/file\". I'm in the \"src\" directory and want to\n> see the \"file\" at specific revision.\n> \n> Knowing about git show I'd expect something like this to work:\n> \n> $ git show master -- subdir/file\n> \n> But it shows nothing (no output, no warning).\n\n...because you are asking git to show the commit master, filtered by\nsubdir/file, and if that file is not changed there, the commit is not\nselected. If it is changed there, it shows you only the diff affecting\nthat file. (We might want to change this to filter the diff only.)\n\n> Following also does not\n> work as expected:\n> \n> $ git show master:subdir/file\n> fatal: Path 'src/subdir/file' exists, but not 'subdir/file'.\n> Did you mean 'master:src/subdir/file'?\n\nBut git is really understanding about your situation, isn't it? ;)\n\n> \n> Of course following works:\n> \n> $ git show master:src/subdir/file\n> \n> but it's not very convenient to have to specify full path, and it's not what\n> you would expect given that most other commands accept \"-- relativepath\"\n> syntax.\n\nIt's not the command in this case, but the \"commit:pathspec\" syntax, and\nfor every command which understands it, it is relative to root (i.e.\nexpects a full path). But we do have a syntax for relative:\n\ngit show master:./subdir/file\n\nCheers,\nMichael\n"},{"id":"164763","messageId":"2590090d32e748932d988dff3897058b909e8358.1301562935.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"4D94322A.8030409@drmicha.warpmail.net","subject":"[PATCH 1/3] t1506: factor out test for \"Did you mean...\"","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T09:17:33Z","receivedAt":"2011-03-31T09:17:33Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"With the current code, it's a \"'\"'\"'\" jungle, and we test only 1 line of\nthe 2 line response. Factor out and test both.\n\nSigned-off-by: Michael J Gruber \n---\n t/t1506-rev-parse-diagnosis.sh | 17 ++++++++++++-----\n 1 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex 9f8adb1..f9cb202 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -6,6 +6,13 @@ exec expected &&\n+\tprintf \"Did you mean '$1:$2$3'?\\n\" >>expected &&\n+\ttest_cmp expected error\n+}\n+\n HASH_file=\n \n test_expect_success 'set up basic repo' '\n@@ -106,7 +113,7 @@ test_expect_success 'incorrect file in sha1:path' '\n \tgrep \"fatal: Path '\"'\"'index-only.txt'\"'\"' exists on disk, but not in '\"'\"'HEAD'\"'\"'.\" error &&\n \t(cd subdir &&\n \t test_must_fail git rev-parse HEAD:file2.txt 2> error &&\n-\t grep \"Did you mean '\"'\"'HEAD:subdir/file2.txt'\"'\"'?\" error )\n+\t test_did_you_mean HEAD subdir/ file2.txt exists )\n '\n \n test_expect_success 'incorrect file in :path and :N:path' '\n@@ -115,14 +122,14 @@ test_expect_success 'incorrect file in :path and :N:path' '\n \ttest_must_fail git rev-parse :1:nothing.txt 2> error &&\n \tgrep \"Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" error &&\n \ttest_must_fail git rev-parse :1:file.txt 2> error &&\n-\tgrep \"Did you mean '\"'\"':0:file.txt'\"'\"'?\" error &&\n+\ttest_did_you_mean \":0\" \"\" file.txt \"is in the index\" \"at stage 1\" &&\n \t(cd subdir &&\n \t test_must_fail git rev-parse :1:file.txt 2> error &&\n-\t grep \"Did you mean '\"'\"':0:file.txt'\"'\"'?\" error &&\n+\t test_did_you_mean \":0\" \"\" file.txt \"is in the index\" \"at stage 1\" &&\n \t test_must_fail git rev-parse :file2.txt 2> error &&\n-\t grep \"Did you mean '\"'\"':0:subdir/file2.txt'\"'\"'?\" error &&\n+\t test_did_you_mean \":0\" subdir/ file2.txt \"is in the index\" &&\n \t test_must_fail git rev-parse :2:file2.txt 2> error &&\n-\t grep \"Did you mean '\"'\"':0:subdir/file2.txt'\"'\"'?\" error) &&\n+\t test_did_you_mean :0 subdir/ file2.txt \"is in the index\") &&\n \ttest_must_fail git rev-parse :disk-only.txt 2> error &&\n \tgrep \"fatal: Path '\"'\"'disk-only.txt'\"'\"' exists on disk, but not in the index.\" error\n '\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164765","messageId":"4ff35194dc52fa969049f555f8d9358cb7ba0c1a.1301562935.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"4D94322A.8030409@drmicha.warpmail.net","subject":"[PATCH 2/3] sha1_name: Suggest commit:./file for path in subdir","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T09:17:34Z","receivedAt":"2011-03-31T09:17:34Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, the \"Did you mean...\" message suggests \"commit:fullpath\"\nonly. Extend this to show the more convenient \"commit:./file\" form also.\n\nSigned-off-by: Michael J Gruber \n---\n sha1_name.c | 11 +++++++----\n t/t1506-rev-parse-diagnosis.sh | 2 +-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex faea58d..69cd6c8 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1012,11 +1012,13 @@ static void diagnose_invalid_sha1_path(const char *prefix,\n \t\tif (!get_tree_entry(tree_sha1, fullname,\n \t\t\t\t sha1, &mode)) {\n \t\t\tdie(\"Path '%s' exists, but not '%s'.\\n\"\n-\t\t\t \"Did you mean '%s:%s'?\",\n+\t\t\t \"Did you mean '%s:%s' aka '%s:./%s'?\",\n \t\t\t fullname,\n \t\t\t filename,\n \t\t\t object_name,\n-\t\t\t fullname);\n+\t\t\t fullname,\n+\t\t\t object_name,\n+\t\t\t filename);\n \t\t}\n \t\tdie(\"Path '%s' does not exist in '%s'\",\n \t\t filename, object_name);\n@@ -1065,9 +1067,10 @@ static void diagnose_invalid_index_path(int stage,\n \t\tif (ce_namelen(ce) == fullnamelen &&\n \t\t !memcmp(ce->name, fullname, fullnamelen))\n \t\t\tdie(\"Path '%s' is in the index, but not '%s'.\\n\"\n-\t\t\t \"Did you mean ':%d:%s'?\",\n+\t\t\t \"Did you mean ':%d:%s' aka ':%d:./%s'?\",\n \t\t\t fullname, filename,\n-\t\t\t ce_stage(ce), fullname);\n+\t\t\t ce_stage(ce), fullname,\n+\t\t\t ce_stage(ce), filename);\n \t}\n \n \tif (!lstat(filename, &st))\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex f9cb202..4a6396f 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -9,7 +9,7 @@ exec expected &&\n-\tprintf \"Did you mean '$1:$2$3'?\\n\" >>expected &&\n+\tprintf \"Did you mean '$1:$2$3'${2:+ aka '$1:./$3'}?\\n\" >>expected &&\n \ttest_cmp expected error\n }\n \n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164764","messageId":"3bee7fb376e2fb498c9634ab2ff5506f8c74a7bc.1301562936.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"4D94322A.8030409@drmicha.warpmail.net","subject":"[RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T09:17:35Z","receivedAt":"2011-03-31T09:17:35Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"By design, \"git show commit -- path\" is not \"git show commit:path\", and\nthere is no reason to change that. But \"git show commit -- path\" simply\nreturns nothing at all \"most of the time\" because it prunes by pathspec\neven though it does not walk commits. This is pretty useless.\n\nSo, turn off pruning (but keep diff limiting of course) so that \"git\nshow commit -- path\" shows the commit message and the diff that the\ncommit introduces to path (filtered by path); only the diff will be\nempty \"most of the time\".\n\nAs an intended side effect, users mistaking \"git show commit -- path\"\nfor \"git show commit:path\" are automatically reminded that they asked\ngit to show a commit, not a blob.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/log.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9db43ed..ea0ddb4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -411,6 +411,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \topt.def = \"HEAD\";\n \topt.tweak = show_rev_tweak_rev;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n+\trev.prune = 0;\n \n \tcount = rev.pending.nr;\n \tobjects = rev.pending.objects;\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164768","messageId":"4D9454D9.6060406@viscovery.net","threadId":"26939","inReplyTo":"3bee7fb376e2fb498c9634ab2ff5506f8c74a7bc.1301562936.git.git@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-03-31T10:18:01Z","receivedAt":"2011-03-31T10:18:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/31/2011 11:17, schrieb Michael J Gruber:\n> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n> there is no reason to change that. But \"git show commit -- path\" simply\n> returns nothing at all \"most of the time\" because it prunes by pathspec\n> even though it does not walk commits. This is pretty useless.\n> \n> So, turn off pruning (but keep diff limiting of course) so that \"git\n> show commit -- path\" shows the commit message and the diff that the\n> commit introduces to path (filtered by path); only the diff will be\n> empty \"most of the time\".\n\nHow does this interfere with git show --walk commit -- path? Will it now\nshow all commits instead of just those that changed path?\n\n-- Hannes\n"},{"id":"164770","messageId":"4D945E3E.9010006@drmicha.warpmail.net","threadId":"26939","inReplyTo":"4D9454D9.6060406@viscovery.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T10:58:06Z","receivedAt":"2011-03-31T10:58:06Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Sixt venit, vidit, dixit 31.03.2011 12:18:\n> Am 3/31/2011 11:17, schrieb Michael J Gruber:\n>> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n>> there is no reason to change that. But \"git show commit -- path\" simply\n>> returns nothing at all \"most of the time\" because it prunes by pathspec\n>> even though it does not walk commits. This is pretty useless.\n>>\n>> So, turn off pruning (but keep diff limiting of course) so that \"git\n>> show commit -- path\" shows the commit message and the diff that the\n>> commit introduces to path (filtered by path); only the diff will be\n>> empty \"most of the time\".\n> \n> How does this interfere with git show --walk commit -- path? Will it now\n> show all commits instead of just those that changed path?\n\nHmpft,\n\ngit show --walk origin/master\nfatal: unrecognized argument: --walk\n\nNo, that is without my patch ;)\n\nIn other words: \"ENOPARSEOPTS in revision.c\", there is no \"--walk\".\n\nMichael\n"},{"id":"164776","messageId":"4D94688B.1070703@viscovery.net","threadId":"26939","inReplyTo":"4D945E3E.9010006@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-03-31T11:42:03Z","receivedAt":"2011-03-31T11:42:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/31/2011 12:58, schrieb Michael J Gruber:\n> Johannes Sixt venit, vidit, dixit 31.03.2011 12:18:\n>> Am 3/31/2011 11:17, schrieb Michael J Gruber:\n>>> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n>>> there is no reason to change that. But \"git show commit -- path\" simply\n>>> returns nothing at all \"most of the time\" because it prunes by pathspec\n>>> even though it does not walk commits. This is pretty useless.\n>>>\n>>> So, turn off pruning (but keep diff limiting of course) so that \"git\n>>> show commit -- path\" shows the commit message and the diff that the\n>>> commit introduces to path (filtered by path); only the diff will be\n>>> empty \"most of the time\".\n>>\n>> How does this interfere with git show --walk commit -- path? Will it now\n>> show all commits instead of just those that changed path?\n> \n> Hmpft,\n> \n> git show --walk origin/master\n> fatal: unrecognized argument: --walk\n> \n> No, that is without my patch ;)\n> \n> In other words: \"ENOPARSEOPTS in revision.c\", there is no \"--walk\".\n\n8-( Oops sorry, and thanks for doing research that I should have done. At\nfirst, I thought it's named --walk because we have --no-walk. But the name\nis --do-walk.\n\nMy question is then:\n\nHow does this interfere with git show --do-walk commit -- path?\n\n-- Hannes\n"},{"id":"164781","messageId":"4D946E85.9030405@drmicha.warpmail.net","threadId":"26939","inReplyTo":"4D94688B.1070703@viscovery.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T12:07:33Z","receivedAt":"2011-03-31T12:07:33Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Sixt venit, vidit, dixit 31.03.2011 13:42:\n> Am 3/31/2011 12:58, schrieb Michael J Gruber:\n>> Johannes Sixt venit, vidit, dixit 31.03.2011 12:18:\n>>> Am 3/31/2011 11:17, schrieb Michael J Gruber:\n>>>> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n>>>> there is no reason to change that. But \"git show commit -- path\" simply\n>>>> returns nothing at all \"most of the time\" because it prunes by pathspec\n>>>> even though it does not walk commits. This is pretty useless.\n>>>>\n>>>> So, turn off pruning (but keep diff limiting of course) so that \"git\n>>>> show commit -- path\" shows the commit message and the diff that the\n>>>> commit introduces to path (filtered by path); only the diff will be\n>>>> empty \"most of the time\".\n>>>\n>>> How does this interfere with git show --walk commit -- path? Will it now\n>>> show all commits instead of just those that changed path?\n>>\n>> Hmpft,\n>>\n>> git show --walk origin/master\n>> fatal: unrecognized argument: --walk\n>>\n>> No, that is without my patch ;)\n>>\n>> In other words: \"ENOPARSEOPTS in revision.c\", there is no \"--walk\".\n> \n> 8-( Oops sorry, and thanks for doing research that I should have done. At\n> first, I thought it's named --walk because we have --no-walk. But the name\n> is --do-walk.\n> \n> My question is then:\n> \n> How does this interfere with git show --do-walk commit -- path?\n\nUh, didn't know that one. If I had thought we have a consistent\ninterface before...\n\nSo, yes, it changes that case also. Note that nothing in git-show[1]\nhints at the fact that one can use rev-list arguments, especially that\none could walk the commits, so it is undocumented behavior which would\nchange.\n\nI would even say that quite generally it is difficult to find cases\nwhere prune and no_walk make sense together. One may construe a user who\nfeeds a list of commits to \"rev-list --no-walk\" and uses it as a filter,\nreturning only those commits which touch a given pathspec. But, I'm not\nsuggesting to change \"rev-list --no-walk -- pathspec\" or log.\n\nBut I do think that \"show\" would benefit from a better default. An\nalternative would be to imply --sparse (i.e. dense=0).\n\nBTW: If you look at cmd_log_reflog() you see that already for other\nlog-like commands we have the problem that we have to override the\nlog-centric defaults from cmd_log_init(), which means ignoring certain\nuser specified rev-list and diff-core options!\n\nMichael\n"},{"id":"164787","messageId":"AANLkTik4wy3B1S=7_2opLdAVy5LBq55VsfZnkj0=QskC@mail.gmail.com","threadId":"26939","inReplyTo":"3bee7fb376e2fb498c9634ab2ff5506f8c74a7bc.1301562936.git.git@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-31T12:50:46Z","receivedAt":"2011-03-31T12:50:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I suggest add \"commit\" to the subject: do not prune commit by\npathspec. Diff limiting is another kind of pruning and I was confused.\n\nOn Thu, Mar 31, 2011 at 4:17 PM, Michael J Gruber\n wrote:\n> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n> there is no reason to change that.\n\nEven more true, now that \"path\" can be \"*.sh\".\n\n> But \"git show commit -- path\" simply\n> returns nothing at all \"most of the time\" because it prunes by pathspec\n> even though it does not walk commits. This is pretty useless.\n>\n> So, turn off pruning (but keep diff limiting of course) so that \"git\n> show commit -- path\" shows the commit message and the diff that the\n> commit introduces to path (filtered by path); only the diff will be\n> empty \"most of the time\".\n\nTests please?\n\n> As an intended side effect, users mistaking \"git show commit -- path\"\n> for \"git show commit:path\" are automatically reminded that they asked\n> git to show a commit, not a blob.\n\nNor a tree. I don't really see how \"git show commit:path\" and \"git how\ncommit -- path\" are relevant for it to be mentioned here.\n-- \nDuy\n"},{"id":"164791","messageId":"4D948105.3050009@drmicha.warpmail.net","threadId":"26939","inReplyTo":"AANLkTik4wy3B1S=7_2opLdAVy5LBq55VsfZnkj0=QskC@mail.gmail.com","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T13:26:29Z","receivedAt":"2011-03-31T13:26:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 31.03.2011 14:50:\n> I suggest add \"commit\" to the subject: do not prune commit by\n> pathspec. Diff limiting is another kind of pruning and I was confused.\n\nI know that under the name \"diff limiting\" only (that may be my ignorance).\n\n> \n> On Thu, Mar 31, 2011 at 4:17 PM, Michael J Gruber\n> wrote:\n>> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n>> there is no reason to change that.\n> \n> Even more true, now that \"path\" can be \"*.sh\".\n> \n>> But \"git show commit -- path\" simply\n>> returns nothing at all \"most of the time\" because it prunes by pathspec\n>> even though it does not walk commits. This is pretty useless.\n>>\n>> So, turn off pruning (but keep diff limiting of course) so that \"git\n>> show commit -- path\" shows the commit message and the diff that the\n>> commit introduces to path (filtered by path); only the diff will be\n>> empty \"most of the time\".\n> \n> Tests please?\n\nHeck, we don't have any to begin with, and this is marked RFC. Given our\nusual reluctance to change even undocumented behavior I'm not going to\nbother with tests for an RFC.\n\n>> As an intended side effect, users mistaking \"git show commit -- path\"\n>> for \"git show commit:path\" are automatically reminded that they asked\n>> git to show a commit, not a blob.\n> \n> Nor a tree. I don't really see how \"git show commit:path\" and \"git how\n> commit -- path\" are relevant for it to be mentioned here.\n\n\"git show commit:path\" is relevant because that is what the OP was\ntrying to do, and \"git show commit:path\" is relevant because that is\nwhat the OP tried and confused him because there was no output at all.\nNot to mention that this is the command this patch is about.\n\nSomeone needs a coffee, me thinks ;)\n\nMichael\n"},{"id":"164792","messageId":"AANLkTi=S9WUWHE8DZvQ2sxMLAJigi1wm3iU_PL5h4Mzd@mail.gmail.com","threadId":"26939","inReplyTo":"4D948105.3050009@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-31T13:35:47Z","receivedAt":"2011-03-31T13:35:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 31, 2011 at 8:26 PM, Michael J Gruber\n wrote:\n>> Tests please?\n>\n> Heck, we don't have any to begin with, and this is marked RFC. Given our\n> usual reluctance to change even undocumented behavior I'm not going to\n> bother with tests for an RFC.\n\nUgh.. I missed the \"RFC\" part on the subject. Sorry.\n\n>>> As an intended side effect, users mistaking \"git show commit -- path\"\n>>> for \"git show commit:path\" are automatically reminded that they asked\n>>> git to show a commit, not a blob.\n>>\n>> Nor a tree. I don't really see how \"git show commit:path\" and \"git how\n>> commit -- path\" are relevant for it to be mentioned here.\n>\n> \"git show commit:path\" is relevant because that is what the OP was\n> trying to do, and \"git show commit:path\" is relevant because that is\n> what the OP tried and confused him because there was no output at all.\n> Not to mention that this is the command this patch is about.\n\nThanks. I see I also missed the original thread somewhere. Someone\nneeds a sleep, me thinks :)\n-- \nDuy\n"},{"id":"164794","messageId":"4D9487EA.4090507@drmicha.warpmail.net","threadId":"26939","inReplyTo":"AANLkTi=S9WUWHE8DZvQ2sxMLAJigi1wm3iU_PL5h4Mzd@mail.gmail.com","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-31T13:55:54Z","receivedAt":"2011-03-31T13:55:54Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 31.03.2011 15:35:\n> On Thu, Mar 31, 2011 at 8:26 PM, Michael J Gruber\n> wrote:\n>>> Tests please?\n>>\n>> Heck, we don't have any to begin with, and this is marked RFC. Given our\n>> usual reluctance to change even undocumented behavior I'm not going to\n>> bother with tests for an RFC.\n> \n> Ugh.. I missed the \"RFC\" part on the subject. Sorry.\n> \n>>>> As an intended side effect, users mistaking \"git show commit -- path\"\n>>>> for \"git show commit:path\" are automatically reminded that they asked\n>>>> git to show a commit, not a blob.\n>>>\n>>> Nor a tree. I don't really see how \"git show commit:path\" and \"git how\n>>> commit -- path\" are relevant for it to be mentioned here.\n>>\n>> \"git show commit:path\" is relevant because that is what the OP was\n>> trying to do, and \"git show commit:path\" is relevant because that is\n>> what the OP tried and confused him because there was no output at all.\n>> Not to mention that this is the command this patch is about.\n> \n> Thanks. I see I also missed the original thread somewhere. Someone\n> needs a sleep, me thinks :)\n\nI'm trying with the coffee that I had suggested (seems to be working),\nbut I guess that decision (between coffee and sleep) depends on the\ntimezone also :)\n\nMichael\n"},{"id":"164823","messageId":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","threadId":"26939","inReplyTo":"4D948105.3050009@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-31T19:23:24Z","receivedAt":"2011-03-31T19:23:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>> Tests please?\n>\n> Heck, we don't have any to begin with, and this is marked RFC. Given our\n> usual reluctance to change even undocumented behavior I'm not going to\n> bother with tests for an RFC.\n\nQuite the contrary, a well written test is a concise and readable way to\nillustrate what behaviour the proposed change is making, and helps judging\nif it is going in a good direction. So if it is an RFC, a test would help\nvery much, especially if there isn't any in the area currently.\n"},{"id":"164824","messageId":"7vsju3jdm2.fsf@alter.siamese.dyndns.org","threadId":"26939","inReplyTo":"4ff35194dc52fa969049f555f8d9358cb7ba0c1a.1301562935.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/3] sha1_name: Suggest commit:./file for path in subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-31T19:26:13Z","receivedAt":"2011-03-31T19:26:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, the \"Did you mean...\" message suggests \"commit:fullpath\"\n> only. Extend this to show the more convenient \"commit:./file\" form also.\n\nIf we were to do this, I suspect that with non-empty prefix we should only\nshow \"./$file\" form for brevity without aka. This is a end-user facing\nmessage and not meant to help scripts, no?\n"},{"id":"164829","messageId":"AANLkTimkdRtY_2S1U=Kog08EOAx_U2cgAFdEC60AZ4eO@mail.gmail.com","threadId":"26939","inReplyTo":"4D94322A.8030409@drmicha.warpmail.net","subject":"Re: Usability improvement request: git show revision -- file","fromName":"Piotr Krukowiecki","fromEmail":"piotr.krukowiecki@gmail.com","sentAt":"2011-03-31T19:59:24Z","receivedAt":"2011-03-31T19:59:24Z","isPatch":false,"sender":{"key":"piotr.krukowiecki@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3259959?v=4"},"body":"On Thu, Mar 31, 2011 at 9:50 AM, Michael J Gruber\n wrote:\n> Piotr Krukowiecki venit, vidit, dixit 31.03.2011 08:45:\n>> Hi,\n>>\n>> if there's existing way to do this please tell me.\n>>\n>> There's this file \"src/subdir/file\". I'm in the \"src\" directory and want to\n>> see the \"file\" at specific revision.\n>>\n>> Knowing about git show I'd expect something like this to work:\n>>\n>>    $ git show master -- subdir/file\n>>\n>> But it shows nothing (no output, no warning).\n>\n> ...because you are asking git to show the commit master, filtered by\n> subdir/file, and if that file is not changed there, the commit is not\n\nIt wasn't - I was doing git-blame and wanted to see file as it was before\nthe blamed commit.\n\n\n> selected. If it is changed there, it shows you only the diff affecting\n\nI see - that makes sense too.\n\n\n> that file. (We might want to change this to filter the diff only.)\n\nNot sure what you mean by that?\n\n\n>> Following also does not\n>> work as expected:\n>>\n>>    $ git show master:subdir/file\n>>    fatal: Path 'src/subdir/file' exists, but not 'subdir/file'.\n>>    Did you mean 'master:src/subdir/file'?\n>\n> But git is really understanding about your situation, isn't it? ;)\n\nYeah, but this looks like a workround for a common problem -\npeople specify relative path a lot, so a warning was added.\n\n\n>> Of course following works:\n>>\n>>    $ git show master:src/subdir/file\n>>\n>> but it's not very convenient to have to specify full path, and it's not what\n>> you would expect given that most other commands accept \"-- relativepath\"\n>> syntax.\n>\n> It's not the command in this case, but the \"commit:pathspec\" syntax, and\n> for every command which understands it, it is relative to root (i.e.\n> expects a full path). But we do have a syntax for relative:\n>\n> git show master:./subdir/file\n\nThanks, I'll try to remember. I don't have a better idea for solving\nthis problem.\n\n\n-- \nPiotr Krukowiecki\n"},{"id":"164855","messageId":"4D9574CC.7090005@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T06:46:36Z","receivedAt":"2011-04-01T06:46:36Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 31.03.2011 21:23:\n> Michael J Gruber writes:\n> \n>>> Tests please?\n>>\n>> Heck, we don't have any to begin with, and this is marked RFC. Given our\n>> usual reluctance to change even undocumented behavior I'm not going to\n>> bother with tests for an RFC.\n> \n> Quite the contrary, a well written test is a concise and readable way to\n> illustrate what behaviour the proposed change is making, and helps judging\n> if it is going in a good direction. So if it is an RFC, a test would help\n> very much, especially if there isn't any in the area currently.\n\nWhile that may be true in some cases (e.g., providing sample output) I\ndon't think the commit message to 3/3 leaves anything open that a test\ncould clarify.\n\nMichael\n"},{"id":"164856","messageId":"4D957648.8070008@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vsju3jdm2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] sha1_name: Suggest commit:./file for path in subdir","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T06:52:56Z","receivedAt":"2011-04-01T06:52:56Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 31.03.2011 21:26:\n> Michael J Gruber writes:\n> \n>> Currently, the \"Did you mean...\" message suggests \"commit:fullpath\"\n>> only. Extend this to show the more convenient \"commit:./file\" form also.\n> \n> If we were to do this, I suspect that with non-empty prefix we should only\n> show \"./$file\" form for brevity without aka. This is a end-user facing\n> message and not meant to help scripts, no?\n\nENOPARSE\n\nDo you mean:\n\n- replace the old \"commit:fullpath\" with \"commit:./file\" or\n\n- show the new form only without \"aka\" (but with the old form) or\n\n- show literal \"commit:./$file\"?\n\nI guess you meant the first one. But I left in both forms on purpose:\nSaying only \"commit:./file\" does not explain what it means, the \"./\"\npart is easy to miss, and the user may not even be aware to be in a\nsubdir. Listing both does not take much space and explains everything.\n\nMichael\n"},{"id":"164868","messageId":"cover.1301649372.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/4] reflog, show and command line overrides","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T09:20:30Z","receivedAt":"2011-04-01T09:20:30Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"While thinking about how to redo 3/3 (show: do not prune by pathspec) I\nnoticed a somehow related reflog problem, which overrides some command line\noptions.\n\nSo, here is some refactoring, a test exposing the reflog problem, and a fix for\nreflog (the new 1/4 through 3/4). Those 3 should be general good cleanup.\n\nIt turned out that the refactoring does not help with the show problem, but I\nchanged the old 3/3 so that we change the pruning by commits only when the user\nhas not requested to walk with show (the new 4/4). No time for new test now, sorry.\n\nThe old 1/3 and 2/3 (\"Did you mean...\") are not impacted (and not resent). They\nmake for independent good UI cleanup also (and were related thematically only,\nnot technically).\n\nMichael J Gruber (4):\n builtin/log.c: separate default and setup of cmd_log_init()\n t/t1411: test reflog with formats\n reflog: fix overriding of command line options\n builtin/show: do not prune by pathspec\n\n builtin/log.c | 32 +++++++++++++++++++-------------\n t/t1411-reflog-show.sh | 18 ++++++++++++++++++\n 2 files changed, 37 insertions(+), 13 deletions(-)\n\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164869","messageId":"3872d568e82874257c0bce931ef3051b96914671.1301649372.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/4] builtin/log.c: separate default and setup of cmd_log_init()","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T09:20:31Z","receivedAt":"2011-04-01T09:20:31Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"cmd_log_init() sets up some default rev options and then calls\nsetup_revisions(), so that a caller cannot set up own defaults: Either\nthey get overriden by cmd_log_init() (if set before) or they override\nthe command line (if set after). We even complain about this in a\ncomment to cmd_log_reflog().\n\nTherefore, separate the two steps so that one can still call\ncmd_log_init() or, alternatively, cmd_log_init_defaults() followed by\ncmd_log_init_finish() (and set defaults in between).\n\nNo functional change so far.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/log.c | 21 +++++++++++++++------\n 1 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 9db43ed..f585209 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -49,13 +49,8 @@ static int parse_decoration_style(const char *var, const char *value)\n \treturn -1;\n }\n \n-static void cmd_log_init(int argc, const char **argv, const char *prefix,\n-\t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n+static void cmd_log_init_defaults(struct rev_info *rev)\n {\n-\tint i;\n-\tint decoration_given = 0;\n-\tstruct userformat_want w;\n-\n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n \tif (fmt_pretty)\n@@ -68,7 +63,14 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \n \tif (default_date_mode)\n \t\trev->date_mode = parse_date_format(default_date_mode);\n+}\n \n+static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n+\t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n+{\n+\tint i;\n+\tint decoration_given = 0;\n+\tstruct userformat_want w;\n \t/*\n \t * Check for -h before setup_revisions(), or \"git log -h\" will\n \t * fail when run without a git directory.\n@@ -128,6 +130,13 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tsetup_pager();\n }\n \n+static void cmd_log_init(int argc, const char **argv, const char *prefix,\n+\t\t\t struct rev_info *rev, struct setup_revision_opt *opt)\n+{\n+\tcmd_log_init_defaults(rev);\n+\tcmd_log_init_finish(argc, argv, prefix, rev, opt);\n+}\n+\n /*\n * This gives a rough estimate for how many commits we\n * will print out in the list.\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164871","messageId":"60228d6916ed82bb45429fb4fed80f39f4e92a6b.1301649372.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/4] t/t1411: test reflog with formats","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T09:20:32Z","receivedAt":"2011-04-01T09:20:32Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"\"git reflog --format=short\" does not work because \"reflog\" overrides the\nformat option. This is documented in code. Document this by a test\n(known failure) also.\n\nSigned-off-by: Michael J Gruber \n---\n t/t1411-reflog-show.sh | 18 ++++++++++++++++++\n 1 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh\nindex ba25ff3..88dc6a7 100755\n--- a/t/t1411-reflog-show.sh\n+++ b/t/t1411-reflog-show.sh\n@@ -28,6 +28,24 @@ test_expect_success 'oneline reflog format' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'reflog default format' '\n+\tgit reflog -1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<'EOF'\n+commit e46513e\n+Reflog: HEAD@{0} (C O Mitter )\n+Reflog message: commit (initial): one\n+Author: A U Thor \n+\n+ one\n+EOF\n+test_expect_failure 'override reflog default format' '\n+\tgit reflog --format=short -1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n cat >expect <<'EOF'\n Reflog: HEAD@{Thu Apr 7 15:13:13 2005 -0700} (C O Mitter )\n Reflog message: commit (initial): one\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164872","messageId":"d89cd4bda86934489f931858b8ecaf2e8525c7ad.1301649372.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","subject":"[PATCH 3/4] reflog: fix overriding of command line options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T09:20:33Z","receivedAt":"2011-04-01T09:20:33Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, \"git reflog\" overrides some command line options such as\n\"--format\".\n\nFix this by using the new 2-phase version of cmd_log_init().\n\nSigned-off-by: Michael J Gruber \n---\n builtin/log.c | 9 ++-------\n t/t1411-reflog-show.sh | 2 +-\n 2 files changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f585209..916019c 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -495,16 +495,11 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \trev.verbose_header = 1;\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n-\tcmd_log_init(argc, argv, prefix, &rev, &opt);\n-\n-\t/*\n-\t * This means that we override whatever commit format the user gave\n-\t * on the cmd line. Sad, but cmd_log_init() currently doesn't\n-\t * allow us to set a different default.\n-\t */\n+\tcmd_log_init_defaults(&rev);\n \trev.commit_format = CMIT_FMT_ONELINE;\n \trev.use_terminator = 1;\n \trev.always_show_header = 1;\n+\tcmd_log_init_finish(argc, argv, prefix, &rev, &opt);\n \n \treturn cmd_log_walk(&rev);\n }\ndiff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh\nindex 88dc6a7..caa687b 100755\n--- a/t/t1411-reflog-show.sh\n+++ b/t/t1411-reflog-show.sh\n@@ -41,7 +41,7 @@ Author: A U Thor \n \n one\n EOF\n-test_expect_failure 'override reflog default format' '\n+test_expect_success 'override reflog default format' '\n \tgit reflog --format=short -1 >actual &&\n \ttest_cmp expect actual\n '\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164870","messageId":"6509d23c538dfabd4fac8870d29a69b1dd11a516.1301649372.git.git@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7vwrjfjdqr.fsf@alter.siamese.dyndns.org","subject":"[PATCH 4/4] builtin/show: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-01T09:20:34Z","receivedAt":"2011-04-01T09:20:34Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"By design, \"git show commit -- path\" is not \"git show commit:path\", and\nthere is no reason to change that. But \"git show commit -- path\" simply\nreturns nothing at all \"most of the time\" because it prunes by pathspec\neven though it does not walk commits. This is pretty useless.\n\nSo, turn off commit pruning (but keep diff limiting of course) so that\n\"git show commit -- path\" shows the commit message and the diff that the\ncommit introduces to path (filtered by path); only the diff will be\nempty \"most of the time\".\n\nAs an intended side effect, users mistaking \"git show commit -- path\"\nfor \"git show commit:path\" are automatically reminded that they asked\ngit to show a commit, not a blob.\n\nIn case the user has specified \"--do-walk\", assume they want the old\nbehaviour (prune by default).\n\nSigned-off-by: Michael J Gruber \n---\n builtin/log.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 916019c..474a76d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -420,6 +420,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \topt.def = \"HEAD\";\n \topt.tweak = show_rev_tweak_rev;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n+\tif (rev.no_walk)\n+\t\trev.prune = 0;\n \n \tcount = rev.pending.nr;\n \tobjects = rev.pending.objects;\n-- \n1.7.4.2.668.gba03a4\n"},{"id":"164909","messageId":"7vfwq1g51y.fsf@alter.siamese.dyndns.org","threadId":"26939","inReplyTo":"4D957648.8070008@drmicha.warpmail.net","subject":"Re: [PATCH 2/3] sha1_name: Suggest commit:./file for path in subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-01T19:11:37Z","receivedAt":"2011-04-01T19:11:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Junio C Hamano venit, vidit, dixit 31.03.2011 21:26:\n>> Michael J Gruber writes:\n>> \n>>> Currently, the \"Did you mean...\" message suggests \"commit:fullpath\"\n>>> only. Extend this to show the more convenient \"commit:./file\" form also.\n>> \n>> If we were to do this, I suspect that with non-empty prefix we should only\n>> show \"./$file\" form for brevity without aka. This is a end-user facing\n>> message and not meant to help scripts, no?\n>\n> ENOPARSE\n\nSorry. What I meant was to show only \"$commit:./$file\" (with $commit and\n$file substituted appropriately so that the user can cut&paste from the\noutput) when we issue this advice in a subdirectory.\n\nThis is meant to be a direct and immediate advice to the user who typed it\nfrom the command line to tell what he might meant. It is beneficial to\nteach the user that there are two different ways to spell it, and that in\ndifferent contexts one of these two ways may be easier to use over the\nother way, but this is not a place to do so.\n\nWe know the context and what the user wanted to do (iow, the command is\nrun in a subdirectory, and we already guessed that the user probably\nwanted to name the path in that subdirectory), so we already know between\ntwo forms, ./$path_around_here form, not the $prefix/$path_around_here\nform, is more appropriate, no?\n"},{"id":"164930","messageId":"7vwrjdej42.fsf@alter.siamese.dyndns.org","threadId":"26939","inReplyTo":"6509d23c538dfabd4fac8870d29a69b1dd11a516.1301649372.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 4/4] builtin/show: do not prune by pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-01T21:50:53Z","receivedAt":"2011-04-01T21:50:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> By design, \"git show commit -- path\" is not \"git show commit:path\", and\n> there is no reason to change that. But \"git show commit -- path\" simply\n> returns nothing at all \"most of the time\" because it prunes by pathspec\n> even though it does not walk commits. This is pretty useless.\n\nHmm, I'm very tempted to suggest throwing it into the \"don't do it then\"\nbasket.\n\n> As an intended side effect, users mistaking \"git show commit -- path\"\n> for \"git show commit:path\" are automatically reminded that they asked\n> git to show a commit, not a blob.\n>\n> In case the user has specified \"--do-walk\", assume they want the old\n> behaviour (prune by default).\n\nCompared to \"--do-walk\", \"git show HEAD~5..\" would be a much more common\nway to trigger it, and is a more appropriate justification why you made\nthe non-pruning conditional.\n\n> Signed-off-by: Michael J Gruber \n> ---\n> builtin/log.c | 2 ++\n> 1 files changed, 2 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 916019c..474a76d 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -420,6 +420,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n> \topt.def = \"HEAD\";\n> \topt.tweak = show_rev_tweak_rev;\n> \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n> +\tif (rev.no_walk)\n> +\t\trev.prune = 0;\n> \n> \tcount = rev.pending.nr;\n> \tobjects = rev.pending.objects;\n"},{"id":"164935","messageId":"7v7hbdefy3.fsf@alter.siamese.dyndns.org","threadId":"26939","inReplyTo":"7vwrjdej42.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin/show: do not prune by pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-01T22:59:16Z","receivedAt":"2011-04-01T22:59:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I attempted to rewrite the log message in a bit more objective voice like\nthis:\n\n builtin/show: do not prune by pathspec\n \n \"git show $commit -- $path\" does not show anything for a commit that does\n not change the $path. While this may be technically correct, it is somewhat\n unexpected from the end user's point of view.\n \n Unless \"show\" is used as \"log -p\", e.g. \"git show HEAD~5..\", it makes more\n sense to show at least the log message for commits, even they are\n uninteresting with respect to $path.\n \n Turn off commit pruning (but keep diff limiting of course) so that the\n command shows the log message and the diff that the commit introduces to\n the path. The diff part may be empty for a given commit that does not\n touch the path.\n \n As an intended side effect, users mistaking \"git show commit -- path\"\n for \"git show commit:path\" are automatically reminded that they asked\n git to show a commit, not a blob.\n \n Signed-off-by: Michael J Gruber \n Signed-off-by: Junio C Hamano \n\nwhich made me realize that this change does regress for no-walk case.\n\nWhat if you (or your homegrown tool or alias) are feeding a list of\ncandidate commits that may have touched the path, without walking, and are\nexpecting them to be filtered?\n\n $ git show A B C D -- path\n\nWe used to get a nice output of \"git show C -- path\" in such a case but\nnow the output will be cluttered with the log message from a commit that\nis totally uninteresting with respect to the given path.\n\nI really wanted to like this patch, because I _very much_ liked the\n\"intended\" ;-) side effect.\n\nI am torn.\n"},{"id":"164991","messageId":"4D98732E.5010709@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7v7hbdefy3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin/show: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-03T13:16:30Z","receivedAt":"2011-04-03T13:16:30Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 02.04.2011 00:59:\n> I attempted to rewrite the log message in a bit more objective voice like\n> this:\n> \n> builtin/show: do not prune by pathspec\n> \n> \"git show $commit -- $path\" does not show anything for a commit that does\n> not change the $path. While this may be technically correct, it is somewhat\n> unexpected from the end user's point of view.\n> \n> Unless \"show\" is used as \"log -p\", e.g. \"git show HEAD~5..\", it makes more\n> sense to show at least the log message for commits, even they are\n> uninteresting with respect to $path.\n\nDefinitely more diplomatic ;)\n\n> Turn off commit pruning (but keep diff limiting of course) so that the\n> command shows the log message and the diff that the commit introduces to\n> the path. The diff part may be empty for a given commit that does not\n> touch the path.\n> \n> As an intended side effect, users mistaking \"git show commit -- path\"\n> for \"git show commit:path\" are automatically reminded that they asked\n> git to show a commit, not a blob.\n> \n> Signed-off-by: Michael J Gruber \n> Signed-off-by: Junio C Hamano \n> \n> which made me realize that this change does regress for no-walk case.\n\nWell, it's an intended change of behaviour, so the term \"regress\" is\ndiplomatically problematic...\n\n> \n> What if you (or your homegrown tool or alias) are feeding a list of\n> candidate commits that may have touched the path, without walking, and are\n> expecting them to be filtered?\n> \n> $ git show A B C D -- path\n> \n> We used to get a nice output of \"git show C -- path\" in such a case but\n> now the output will be cluttered with the log message from a commit that\n> is totally uninteresting with respect to the given path.\n> \n> I really wanted to like this patch, because I _very much_ liked the\n> \"intended\" ;-) side effect.\n> \n> I am torn.\n> \n\nI would probably discard the \"tool\" aspect (for a porcelain). FWIW, we\nnever even documented that \"git show\" takes pathspec arguments, and only\nexperienced users know that \"git show\" has anything to do with \"git log\"\nand friends (we mention only the relation to diff-tree).\n\nThat being said, we could detect in addition whether there is only one\npositive rev (and no walk being done) and turn off pruning only in that\ncase. That would still catch users' (mis)expectation about \"show rev --\nfile\" but not impact anyone using it as a filter on more than 1 rev.\n(Personally, I find the distinction between walk and no-walk modes clearer.)\n\nMichael\n"},{"id":"165137","messageId":"7v1v1h8z5k.fsf@alter.siamese.dyndns.org","threadId":"26939","inReplyTo":"6509d23c538dfabd4fac8870d29a69b1dd11a516.1301649372.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 4/4] builtin/show: do not prune by pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-04T21:49:59Z","receivedAt":"2011-04-04T21:49:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 916019c..474a76d 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -420,6 +420,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n> \topt.def = \"HEAD\";\n> \topt.tweak = show_rev_tweak_rev;\n> \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n> +\tif (rev.no_walk)\n> +\t\trev.prune = 0;\n\nThis is not your fault, but I am somewhat disgusted by the reason why\n\n $ git show master..next [ -- Documentation ]\n\nworks by \"walking\" the history. It takes completely different codepath\nfrom \"git log\" with the same set of arguments.\n\n * first log_init() grabs '^master' and 'next' into the rev.pending object\n array;\n\n * we pop '^master' first, make it the only object in the rev.pending\n object array, and let cmd_log_walk() call prepare_revision_walk() on it\n to limit the list. Since we don't have any positive ref, we get\n nothing;\n\n * then we pop 'next', make it again _the only_ object in the rev.pending\n object array; prepare_revision_walk() on the same codepath now limits\n the list to exclude what is reachable from 'master', only because we\n have processed '^master'.\n\nYikes. In other words, the reason the current code works is only by\naccident.\n\nThis is a tangent, but we _might_ want to (perhaps at git 2.0) update the\nrevision parsing machinery so that we can clear the object flags more\neasily and have\n\n $ git log A..B C..D [ -- pathspec ]\n\ncompute the union of (commits reachable from B but not from A) and\n(commits reachable from D but not from C). I say we _might_ because we\nwould still want to compute what the current code computes in some cases,\nand we may be able to express it as \"^A ^C B D\", but I am not sure if we\nwant to go that route and end up with a general set operation (with unions\nand intersections, perhaps even using parentheses to express precedence).\n\nMore general set operation is certainly doable, and at that point it\nprobably does not matter that we cannot stream the output as we are doing\nsomething complex (IOW, we would be revs->limited, giving up the latency),\nbut I don't know if it is useful in the first place to support such\ngeneral case. Because the most complex set operations that I run every\nday is\n\n $ git log ^master $topic1 $topic2 $topic3... -- $pathspec\n\nand I don't recall cases in which I wished A..B C..D showed commits\nwithin A..B that are reachable from C.\n"},{"id":"165167","messageId":"4D9AB17B.70409@drmicha.warpmail.net","threadId":"26939","inReplyTo":"7v1v1h8z5k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin/show: do not prune by pathspec","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-05T06:06:51Z","receivedAt":"2011-04-05T06:06:51Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 04.04.2011 23:49:\n> Michael J Gruber writes:\n> \n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 916019c..474a76d 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -420,6 +420,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n>> \topt.def = \"HEAD\";\n>> \topt.tweak = show_rev_tweak_rev;\n>> \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n>> +\tif (rev.no_walk)\n>> +\t\trev.prune = 0;\n> \n> This is not your fault, but I am somewhat disgusted by the reason why\n> \n> $ git show master..next [ -- Documentation ]\n> \n> works by \"walking\" the history. It takes completely different codepath\n> from \"git log\" with the same set of arguments.\n> \n> * first log_init() grabs '^master' and 'next' into the rev.pending object\n> array;\n> \n> * we pop '^master' first, make it the only object in the rev.pending\n> object array, and let cmd_log_walk() call prepare_revision_walk() on it\n> to limit the list. Since we don't have any positive ref, we get\n> nothing;\n> \n> * then we pop 'next', make it again _the only_ object in the rev.pending\n> object array; prepare_revision_walk() on the same codepath now limits\n> the list to exclude what is reachable from 'master', only because we\n> have processed '^master'.\n> \n> Yikes. In other words, the reason the current code works is only by\n> accident.\n\nThat is super ugly, yes.\n\n> \n> This is a tangent, but we _might_ want to (perhaps at git 2.0) update the\n> revision parsing machinery so that we can clear the object flags more\n> easily and have\n> \n> $ git log A..B C..D [ -- pathspec ]\n> \n> compute the union of (commits reachable from B but not from A) and\n> (commits reachable from D but not from C). I say we _might_ because we\n> would still want to compute what the current code computes in some cases,\n> and we may be able to express it as \"^A ^C B D\", but I am not sure if we\n> want to go that route and end up with a general set operation (with unions\n> and intersections, perhaps even using parentheses to express precedence).\n> \n> More general set operation is certainly doable, and at that point it\n> probably does not matter that we cannot stream the output as we are doing\n> something complex (IOW, we would be revs->limited, giving up the latency),\n> but I don't know if it is useful in the first place to support such\n> general case. Because the most complex set operations that I run every\n> day is\n> \n> $ git log ^master $topic1 $topic2 $topic3... -- $pathspec\n> \n> and I don't recall cases in which I wished A..B C..D showed commits\n> within A..B that are reachable from C.\n> \n\nWe seem to promise that one should think in terms of sets, and this\nbreaks with \"A..B\" and \"C...D\" unless one always thinks of them in the\nresolved form \"^A B\" and \"^E C D\" (with E=$(git merge-base C D)).\n\nI also think that \"A...B C\" treats C as the right of that symmetric\nrange (because anything non-left we treat as right), and \"A...B ^C\"\nprobably interacts badly with --cherry-pick (though I haven't tried).\n\nAlso, \"A...B C...D\" would be a candidate for union. Lot's to do for\nafter 1.7.5...\n\nMichael\n"}]}