{"thread":{"id":"28263","subject":"[PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD","startedAt":"2011-08-30T21:43:03Z","lastAt":"2011-09-06T17:43:24Z","messageCount":3,"participants":["Ron Panduwana","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"174585","messageId":"1314740583-14567-1-git-send-email-panduwana@gmail.com","threadId":"28263","inReplyTo":null,"subject":"[PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD","fromName":"Ron Panduwana","fromEmail":"panduwana@gmail.com","sentAt":"2011-08-30T21:43:03Z","receivedAt":"2011-08-30T21:43:03Z","isPatch":true,"sender":{"key":"panduwana@gmail.com","avatar":"https://gravatar.com/avatar/5fbac94c48ebedfb6415332993323837eeff8420afe2ee9af57f36002801e75e?d=mp&s=160"},"body":"Signed-off-by: Ron Panduwana <panduwana@gmail.com>\n---\n\nOn Fri, Aug 19, 2011 at 5:10 PM, Thomas Rast <trast@student.ethz.ch> wrote:\n> Some thoughts:\n>\n> * running git-status for . has some issues: it doesn't work in the\n>  case of\n>\n>    cd subdir\n>    git add ../some/file[TAB]\n>\n>  It's also inefficient if you are at the top level and\n>\n>    git add path/to/file/a/few/levels/down[TAB]\n>\n>  since it wouldn't actually have to look for untracked files in the\n>  entire repo.\n\nFixed by running git-status for $cur if $cur is a directory. Otherwise run on .\n\n\n> * -uall is not required unless you are looking for untracked files.\n>   For the other commands you could speed up completion by passing\n>   -uno instead.\n\nFixed by adding second parameter to __git_files_having_status\n\n\n contrib/completion/git-completion.bash |   84 ++++++++++++++++++++-----------\n 1 files changed, 54 insertions(+), 30 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 8648a36..9d44501 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1010,6 +1010,16 @@ __git_has_doubledash ()\n \treturn 1\n }\n\n+# __git_files_having_status requires 2 arguments\n+__git_files_having_status ()\n+{\n+    local dir=\".\"\n+    if [ -d \"$cur\" ]; then\n+        dir=\"$cur\"\n+    fi\n+\techo \"$(git status $2 -s \"$dir\" 2>/dev/null | egrep \"^$1\" | cut -c4-)\"\n+}\n+\n __git_whitespacelist=\"nowarn warn error error-all fix\"\n\n _git_am ()\n@@ -1058,17 +1068,17 @@ _git_apply ()\n\n _git_add ()\n {\n-\t__git_has_doubledash && return\n-\n-\tcase \"$cur\" in\n-\t--*)\n-\t\t__gitcomp \"\n-\t\t\t--interactive --refresh --patch --update --dry-run\n-\t\t\t--ignore-errors --intent-to-add\n-\t\t\t\"\n-\t\treturn\n-\tesac\n-\tCOMPREPLY=()\n+\tif ! __git_has_doubledash; then\n+\t\tcase \"$cur\" in\n+\t\t--*)\n+\t\t\t__gitcomp \"\n+\t\t\t\t--interactive --refresh --patch --update --dry-run\n+\t\t\t\t--ignore-errors --intent-to-add\n+\t\t\t\t\"\n+\t\t\treturn\n+\t\tesac\n+\tfi\n+\t__gitcomp \"$(__git_files_having_status \"(.[MAU]|UD|\\?\\?)\" -uall)\"\n }\n\n _git_archive ()\n@@ -1171,7 +1181,12 @@ _git_bundle ()\n\n _git_checkout ()\n {\n-\t__git_has_doubledash && return\n+\tif __git_has_doubledash; then\n+\t\tif [[ ${words[2]} = \"--\" ]]; then\n+\t\t\t__gitcomp \"$(__git_files_having_status \".[MD]\" -uno)\"\n+\t\tfi\n+\t\treturn\n+\tfi\n\n \tcase \"$cur\" in\n \t--conflict=*)\n@@ -1469,7 +1484,7 @@ _git_help ()\n \t__gitcomp \"$__git_all_commands $(__git_aliases)\n \t\tattributes cli core-tutorial cvs-migration\n \t\tdiffcore gitk glossary hooks ignore modules\n-\t\tnamespaces repository-layout tutorial tutorial-2\n+\t\trepository-layout tutorial tutorial-2\n \t\tworkflows\n \t\t\"\n }\n@@ -2313,14 +2328,18 @@ _git_replace ()\n\n _git_reset ()\n {\n-\t__git_has_doubledash && return\n-\n-\tcase \"$cur\" in\n-\t--*)\n-\t\t__gitcomp \"--merge --mixed --hard --soft --patch\"\n+\tif ! __git_has_doubledash; then\n+\t\tcase \"$cur\" in\n+\t\t--*)\n+\t\t\t__gitcomp \"--merge --mixed --hard --soft --patch\"\n+\t\t\treturn\n+\t\t\t;;\n+\t\tesac\n+\tfi\n+\tif [[ ${words[2]} = \"HEAD\" ]]; then\n+\t\t__gitcomp \"$(__git_files_having_status \"[ADM].\" -uno)\"\n \t\treturn\n-\t\t;;\n-\tesac\n+\tfi\n \t__gitcomp \"$(__git_refs)\"\n }\n\n@@ -2337,15 +2356,20 @@ _git_revert ()\n\n _git_rm ()\n {\n-\t__git_has_doubledash && return\n-\n-\tcase \"$cur\" in\n-\t--*)\n-\t\t__gitcomp \"--cached --dry-run --ignore-unmatch --quiet\"\n-\t\treturn\n-\t\t;;\n-\tesac\n-\tCOMPREPLY=()\n+\tif ! __git_has_doubledash; then\n+\t\tcase \"$cur\" in\n+\t\t--*)\n+\t\t\t__gitcomp \"--cached --dry-run --ignore-unmatch --quiet\"\n+\t\t\treturn\n+\t\t\t;;\n+\t\tesac\n+\tfi\n+\t# check if --cached was specified\n+\tif [ \"$(__git_find_on_cmdline \"--cached\")\" ]; then\n+\t\tCOMPREPLY=()\n+\telse\n+\t\t__gitcomp \"$(__git_files_having_status \"(.D|DU|UA)\" -uno)\"\n+\tfi\n }\n\n _git_shortlog ()\n@@ -2640,7 +2664,6 @@ _git ()\n \t\t\t--exec-path\n \t\t\t--html-path\n \t\t\t--work-tree=\n-\t\t\t--namespace=\n \t\t\t--help\n \t\t\t\"\n \t\t\t;;\n@@ -2737,3 +2760,4 @@ else\n \t\tshopt \"$@\"\n \t}\n fi\n+\n--\n1.7.1\n"},{"id":"174588","messageId":"7v1uw21ogf.fsf@alter.siamese.dyndns.org","threadId":"28263","inReplyTo":"1314740583-14567-1-git-send-email-panduwana@gmail.com","subject":"Re: [PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-30T23:14:56Z","receivedAt":"2011-08-30T23:14:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ron Panduwana <panduwana@gmail.com> writes:\n\n> Signed-off-by: Ron Panduwana <panduwana@gmail.com>\n\n\"Make use of\" is something anybody can read from the patch. What we need\nfrom the proposed commit log message above S-o-b: line is to justify why\nit is a good change. Does it make the code simpler to follow by making it\nshorter? Does it make it faster to complete, and if so by how much faster?\nDoes it make it easier to use by not including paths that it used to show,\nand if so what are the differences the end users would see, and why is it\njustified to omit these paths from the candidate set?\n"},{"id":"174943","messageId":"20110906174324.GA8211@goldbirke","threadId":"28263","inReplyTo":"1314740583-14567-1-git-send-email-panduwana@gmail.com","subject":"Re: [PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2011-09-06T17:43:24Z","receivedAt":"2011-09-06T17:43:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\n\nOn Wed, Aug 31, 2011 at 04:43:03AM +0700, Ron Panduwana wrote:\n> Signed-off-by: Ron Panduwana <panduwana@gmail.com>\n\nI agree with Junio that some explanation would be necessary here.\n\nI'm not sure this change is in general a good idea.  I can imagine\nthat it would be useful for e.g. java projects, where you are forced\nto have deep directory structures, but it will surely cause surprises\nand confusion for users who trained their fingers to quickly enter\nlong paths with a few keystrokes and completion.  Furthermore, it\nseems to cause considerable performance penalty, because it will fork\nthree processes and a subshell to connect them with a pipe for such a\n\"simple\" thing as file name completion.  And one of those processes is\ngit status, which alone can take a while, especially for large\nrepositories, and as a side effect it refreshes the index (but that is\nprobably harmless).\n\n> ---\n> \n> On Fri, Aug 19, 2011 at 5:10 PM, Thomas Rast <trast@student.ethz.ch> wrote:\n> > Some thoughts:\n> >\n> > * running git-status for . has some issues: it doesn't work in the\n> >  case of\n> >\n> >    cd subdir\n> >    git add ../some/file[TAB]\n> >\n> >  It's also inefficient if you are at the top level and\n> >\n> >    git add path/to/file/a/few/levels/down[TAB]\n> >\n> >  since it wouldn't actually have to look for untracked files in the\n> >  entire repo.\n> \n> Fixed by running git-status for $cur if $cur is a directory. Otherwise run on .\n\nThat won't do.  Imaginge you want to add ../some/file, and therefore\nyou do 'git add ../so<TAB>'.  In this case $cur will hold '../so',\nwhich is not a directory (or at least not the directory you are\nlooking for).\n\n> > * -uall is not required unless you are looking for untracked files.\n> >   For the other commands you could speed up completion by passing\n> >   -uno instead.\n> \n> Fixed by adding second parameter to __git_files_having_status\n> \n> \n>  contrib/completion/git-completion.bash |   84 ++++++++++++++++++++-----------\n>  1 files changed, 54 insertions(+), 30 deletions(-)\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 8648a36..9d44501 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1010,6 +1010,16 @@ __git_has_doubledash ()\n>  \treturn 1\n>  }\n> \n> +# __git_files_having_status requires 2 arguments\n> +__git_files_having_status ()\n> +{\n> +    local dir=\".\"\n> +    if [ -d \"$cur\" ]; then\n> +        dir=\"$cur\"\n> +    fi\n\nPlease use tabs for indentation.\n\n> +\techo \"$(git status $2 -s \"$dir\" 2>/dev/null | egrep \"^$1\" | cut -c4-)\"\n\nEchoing a command substitution is unnecessary.\n\n> +}\n> +\n>  __git_whitespacelist=\"nowarn warn error error-all fix\"\n> \n>  _git_am ()\n> @@ -1058,17 +1068,17 @@ _git_apply ()\n> \n>  _git_add ()\n>  {\n> -\t__git_has_doubledash && return\n> -\n> -\tcase \"$cur\" in\n> -\t--*)\n> -\t\t__gitcomp \"\n> -\t\t\t--interactive --refresh --patch --update --dry-run\n> -\t\t\t--ignore-errors --intent-to-add\n> -\t\t\t\"\n> -\t\treturn\n> -\tesac\n> -\tCOMPREPLY=()\n> +\tif ! __git_has_doubledash; then\n> +\t\tcase \"$cur\" in\n> +\t\t--*)\n> +\t\t\t__gitcomp \"\n> +\t\t\t\t--interactive --refresh --patch --update --dry-run\n> +\t\t\t\t--ignore-errors --intent-to-add\n> +\t\t\t\t\"\n> +\t\t\treturn\n> +\t\tesac\n> +\tfi\n> +\t__gitcomp \"$(__git_files_having_status \"(.[MAU]|UD|\\?\\?)\" -uall)\"\n\nPlease do this and the others the other way around, as you did in\n_git_checkout(), i.e.\n\n\tif __git_has_doubledash ; then\n\t\t__gitcomp \"$(__git_files_having_status ...)\"\n\t\treturn\n\tfi\n\n\tcase \"$cur\" in\n\t[...]\n\nThis will make the patch shorter and easier to review, it won't\nincrease the indentation unnecesarily, and it will be friendlier to\nthe users running 'git blame' in the future.\n\n>  }\n> \n>  _git_archive ()\n> @@ -1171,7 +1181,12 @@ _git_bundle ()\n> \n>  _git_checkout ()\n>  {\n> -\t__git_has_doubledash && return\n> +\tif __git_has_doubledash; then\n> +\t\tif [[ ${words[2]} = \"--\" ]]; then\n> +\t\t\t__gitcomp \"$(__git_files_having_status \".[MD]\" -uno)\"\n> +\t\tfi\n> +\t\treturn\n> +\tfi\n> \n>  \tcase \"$cur\" in\n>  \t--conflict=*)\n> @@ -1469,7 +1484,7 @@ _git_help ()\n>  \t__gitcomp \"$__git_all_commands $(__git_aliases)\n>  \t\tattributes cli core-tutorial cvs-migration\n>  \t\tdiffcore gitk glossary hooks ignore modules\n> -\t\tnamespaces repository-layout tutorial tutorial-2\n> +\t\trepository-layout tutorial tutorial-2\n\nThat's an independent change; I don't think you want to remove\nnamespaces here.\n\n>  \t\tworkflows\n>  \t\t\"\n>  }\n> @@ -2313,14 +2328,18 @@ _git_replace ()\n> \n>  _git_reset ()\n>  {\n> -\t__git_has_doubledash && return\n> -\n> -\tcase \"$cur\" in\n> -\t--*)\n> -\t\t__gitcomp \"--merge --mixed --hard --soft --patch\"\n> +\tif ! __git_has_doubledash; then\n> +\t\tcase \"$cur\" in\n> +\t\t--*)\n> +\t\t\t__gitcomp \"--merge --mixed --hard --soft --patch\"\n> +\t\t\treturn\n> +\t\t\t;;\n> +\t\tesac\n> +\tfi\n> +\tif [[ ${words[2]} = \"HEAD\" ]]; then\n> +\t\t__gitcomp \"$(__git_files_having_status \"[ADM].\" -uno)\"\n>  \t\treturn\n> -\t\t;;\n> -\tesac\n> +\tfi\n>  \t__gitcomp \"$(__git_refs)\"\n>  }\n> \n> @@ -2337,15 +2356,20 @@ _git_revert ()\n> \n>  _git_rm ()\n>  {\n> -\t__git_has_doubledash && return\n> -\n> -\tcase \"$cur\" in\n> -\t--*)\n> -\t\t__gitcomp \"--cached --dry-run --ignore-unmatch --quiet\"\n> -\t\treturn\n> -\t\t;;\n> -\tesac\n> -\tCOMPREPLY=()\n> +\tif ! __git_has_doubledash; then\n> +\t\tcase \"$cur\" in\n> +\t\t--*)\n> +\t\t\t__gitcomp \"--cached --dry-run --ignore-unmatch --quiet\"\n> +\t\t\treturn\n> +\t\t\t;;\n> +\t\tesac\n> +\tfi\n> +\t# check if --cached was specified\n> +\tif [ \"$(__git_find_on_cmdline \"--cached\")\" ]; then\n> +\t\tCOMPREPLY=()\n> +\telse\n> +\t\t__gitcomp \"$(__git_files_having_status \"(.D|DU|UA)\" -uno)\"\n\n'git rm' can be used to delete any tracked files, but this limits\ncompletion to only those files that are already deleted from the work\ntree or are unmerged.\n\n> +\tfi\n>  }\n> \n>  _git_shortlog ()\n> @@ -2640,7 +2664,6 @@ _git ()\n>  \t\t\t--exec-path\n>  \t\t\t--html-path\n>  \t\t\t--work-tree=\n> -\t\t\t--namespace=\n\nIndependent change.\n\n>  \t\t\t--help\n>  \t\t\t\"\n>  \t\t\t;;\n> @@ -2737,3 +2760,4 @@ else\n>  \t\tshopt \"$@\"\n>  \t}\n>  fi\n> +\n\nSuperfluous new line.\n\n\n\nBest,\nGábor\n"}]}