{"thread":{"id":"32362","subject":"[PATCH] git-completion.bash: update obsolete code.","startedAt":"2012-12-16T21:50:02Z","lastAt":"2012-12-18T01:37:33Z","messageCount":5,"participants":["Manlio Perillo","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"204989","messageId":"1355694602-8771-1-git-send-email-manlio.perillo@gmail.com","threadId":"32362","inReplyTo":null,"subject":"[PATCH] git-completion.bash: update obsolete code.","fromName":"Manlio Perillo","fromEmail":"manlio.perillo@gmail.com","sentAt":"2012-12-16T21:50:02Z","receivedAt":"2012-12-16T21:50:02Z","isPatch":true,"sender":{"key":"manlio.perillo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6217088?v=4"},"body":"The git-completion.bash script was using the git ls-tree command\nwithout the --name-only option, with a sed filter to parse path names;\nuse the --name-only option, instead.\n\nSigned-off-by: Manlio Perillo \n---\n contrib/completion/git-completion.bash | 15 +--------------\n 1 file changed, 1 insertion(+), 14 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 0b77eb1..85d9051 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -397,20 +397,7 @@ __git_complete_revlist_file ()\n \t\t*) pfx=\"$ref:$pfx\" ;;\n \t\tesac\n \n-\t\t__gitcomp_nl \"$(git --git-dir=\"$(__gitdir)\" ls-tree \"$ls\" \\\n-\t\t\t\t| sed '/^100... blob /{\n-\t\t\t\t s,^.*\t,,\n-\t\t\t\t s,$, ,\n-\t\t\t\t }\n-\t\t\t\t /^120000 blob /{\n-\t\t\t\t s,^.*\t,,\n-\t\t\t\t s,$, ,\n-\t\t\t\t }\n-\t\t\t\t /^040000 tree /{\n-\t\t\t\t s,^.*\t,,\n-\t\t\t\t s,$,/,\n-\t\t\t\t }\n-\t\t\t\t s/^.*\t//')\" \\\n+\t\t__gitcomp_nl \"$(git --git-dir=\"$(__gitdir)\" ls-tree --name-only \"$ls\")\" \\\n \t\t\t\"$pfx\" \"$cur_\" \"\"\n \t\t;;\n \t*...*)\n-- \n1.8.1.rc1.18.g9db0d25\n"},{"id":"205004","messageId":"7vtxrlnuqr.fsf@alter.siamese.dyndns.org","threadId":"32362","inReplyTo":"1355694602-8771-1-git-send-email-manlio.perillo@gmail.com","subject":"Re: [PATCH] git-completion.bash: update obsolete code.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T04:54:52Z","receivedAt":"2012-12-17T04:54:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Manlio Perillo writes:\n\n> The git-completion.bash script was using the git ls-tree command\n> without the --name-only option, with a sed filter to parse path names;\n> use the --name-only option, instead.\n>\n> Signed-off-by: Manlio Perillo \n> ---\n\nDid you miss the different handling between blobs and trees the\nlatter gets trailing slash in the completion)?\n\n> contrib/completion/git-completion.bash | 15 +--------------\n> 1 file changed, 1 insertion(+), 14 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 0b77eb1..85d9051 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -397,20 +397,7 @@ __git_complete_revlist_file ()\n> \t\t*) pfx=\"$ref:$pfx\" ;;\n> \t\tesac\n> \n> -\t\t__gitcomp_nl \"$(git --git-dir=\"$(__gitdir)\" ls-tree \"$ls\" \\\n> -\t\t\t\t| sed '/^100... blob /{\n> -\t\t\t\t s,^.*\t,,\n> -\t\t\t\t s,$, ,\n> -\t\t\t\t }\n> -\t\t\t\t /^120000 blob /{\n> -\t\t\t\t s,^.*\t,,\n> -\t\t\t\t s,$, ,\n> -\t\t\t\t }\n> -\t\t\t\t /^040000 tree /{\n> -\t\t\t\t s,^.*\t,,\n> -\t\t\t\t s,$,/,\n> -\t\t\t\t }\n> -\t\t\t\t s/^.*\t//')\" \\\n> +\t\t__gitcomp_nl \"$(git --git-dir=\"$(__gitdir)\" ls-tree --name-only \"$ls\")\" \\\n> \t\t\t\"$pfx\" \"$cur_\" \"\"\n> \t\t;;\n> \t*...*)\n"},{"id":"205062","messageId":"50CF4E05.7050103@gmail.com","threadId":"32362","inReplyTo":"7vtxrlnuqr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-completion.bash: update obsolete code.","fromName":"Manlio Perillo","fromEmail":"manlio.perillo@gmail.com","sentAt":"2012-12-17T16:53:25Z","receivedAt":"2012-12-17T16:53:25Z","isPatch":true,"sender":{"key":"manlio.perillo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6217088?v=4"},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n\nIl 17/12/2012 05:54, Junio C Hamano ha scritto:\n> Manlio Perillo writes:\n> \n>> The git-completion.bash script was using the git ls-tree command\n>> without the --name-only option, with a sed filter to parse path names;\n>> use the --name-only option, instead.\n>>\n>> Signed-off-by: Manlio Perillo \n>> ---\n> \n> Did you miss the different handling between blobs and trees the\n> latter gets trailing slash in the completion)?\n> \n\nYes, I totally missed it, sorry.\nI was assuming the bash completion script was written before --name-only\noption was added to ls-tree.\n\nBy the way, IMHO there should be an option for adding a slash to\ndirectory names in ls-tree.\n\n> [...]\n\n\nRegards Manlio\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v1.4.10 (GNU/Linux)\nComment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/\n\niEUEARECAAYFAlDPTgUACgkQscQJ24LbaUSkKgCePH2NFHf/qp2ZrgI9eD9D0D3G\nzOwAmL8Dc8r9DevyV1ZhaCb2G9MPZxU=\n=QJEy\n-----END PGP SIGNATURE-----\n"},{"id":"205072","messageId":"7v6240l8w6.fsf@alter.siamese.dyndns.org","threadId":"32362","inReplyTo":"50CF4E05.7050103@gmail.com","subject":"Re: [PATCH] git-completion.bash: update obsolete code.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T20:29:45Z","receivedAt":"2012-12-17T20:29:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Manlio Perillo writes:\n\n> By the way, IMHO there should be an option for adding a slash to\n> directory names in ls-tree.\n\nI am not sure about that; ls-tree is meant to be used by scripts\nthat are capable of doing that kind of thing themselves.\n\nIf we were to add an option to add a slash, I think it should be\nmodeled after \"ls -F\", whose output is meant for humans and no sane\nscripts would read from it. It is very likely that such an option\nshould add @ at the end of the name for a symbolic link, so it won't\nbe useful for your patch.\n"},{"id":"205099","messageId":"7v7gogjg2q.fsf@alter.siamese.dyndns.org","threadId":"32362","inReplyTo":"50CF4E05.7050103@gmail.com","subject":"Re: [PATCH] git-completion.bash: update obsolete code.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-18T01:37:33Z","receivedAt":"2012-12-18T01:37:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Manlio Perillo writes:\n\n> Il 17/12/2012 05:54, Junio C Hamano ha scritto:\n>> Manlio Perillo writes:\n>> \n>>> The git-completion.bash script was using the git ls-tree command\n>>> without the --name-only option, with a sed filter to parse path names;\n>>> use the --name-only option, instead.\n>>>\n>>> Signed-off-by: Manlio Perillo \n>>> ---\n>> \n>> Did you miss the different handling between blobs and trees the\n>> latter gets trailing slash in the completion)?\n>> \n>\n> Yes, I totally missed it, sorry.\n> I was assuming the bash completion script was written before --name-only\n> option was added to ls-tree.\n\nThere is no need to say \"sorry\" here; catching this kind of mistake\nis what we have the patch review process for, after all. If\nanything, \"thanks\" is more appropriate ;-)\n\nAnd the way you stated the reasoning behind this change in the\nproposed commit log message was really good. It showed clearly that\nthe patch was an attempt to clean up the code, not was an attempt to\nchange the behaviour to drop the trailing slash or space. If it\nsaid \"let's use ls-tree --name-only\" without such an explanation on\n\"why\", future readers of \"git log\" would have been left scratching\ntheir heads, wondering \"why\" the change was made.\n\nThanks.\n"}]}