{"thread":{"id":"40483","subject":"[PATCH] completion: fix completion after 'git -C path'","startedAt":"2015-10-05T12:02:15Z","lastAt":"2015-10-06T10:12:24Z","messageCount":3,"participants":["SZEDER Gábor","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"271141","messageId":"1444046535-18316-1-git-send-email-szeder@ira.uka.de","threadId":"40483","inReplyTo":null,"subject":"[PATCH] completion: fix completion after 'git -C path'","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-10-05T12:02:15Z","receivedAt":"2015-10-05T12:02:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The main completion function finds the name of the git command by\niterating through all the words on the command line in search for the\nfirst non-option-looking word.  As it is not aware of 'git -C's\nmandatory path argument, if the '-C path' option is present, 'path' will\nbe the first such word and it will be mistaken for a git command.  This\nbreaks the completion script in various ways:\n\n - If 'path' happens to match one of the commands supported by the\n   completion script, then its options will be offered.\n\n - If 'path' doesn't match a supported command and doesn't contain any\n   characters not allowed in Bash identifier names, then the completion\n   script does basically nothing and lets Bash to fall back to filename\n   completion.\n\n - Otherwise, if 'path' contains such unallowed characters, then it\n   leads to a more or less ugly error in the middle of the command line.\n   The standard '/' directory separator is such a character, and it\n   happens to trigger one of the uglier errors:\n\n     $ git -C some/path <TAB>sh.exe\": declare: `_git_some/path': not a valid identifier\n     error: invalid key: alias.some/path\n\nFix this by skipping 'git -C's mandatory path argument while iterating\nover the words on the command line.  Extend the relevant test with this\ncase and, while at it, with cases that needed similar treatment in the\npast ('--git-dir', '-c', '--work-tree' and '--namespace').\nAdditionally, shut up standard error of the 'declare' commands looking\nfor the associated completion function and of the 'git config' query for\nthe aliased command, so if git learns a new option with a mandatory\nargument in the future, then at least the command line will not be\nutterly disrupted by those error messages.\n\nNote, that this change merely fixes the breakage related to 'git -C\npath', but the completion script will not take it into account as it\ndoes '--git-dir path'.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n contrib/completion/git-completion.bash | 8 ++++----\n t/t9902-completion.sh                  | 7 ++++++-\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 482ca84b45..80dc717fe2 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -763,7 +763,7 @@ __git_aliases ()\n __git_aliased_command ()\n {\n \tlocal word cmdline=$(git --git-dir=\"$(__gitdir)\" \\\n-\t\tconfig --get \"alias.$1\")\n+\t\tconfig --get \"alias.$1\" 2>/dev/null)\n \tfor word in $cmdline; do\n \t\tcase \"$word\" in\n \t\t\\!gitk|gitk)\n@@ -2571,7 +2571,7 @@ __git_main ()\n \t\t--git-dir)   ((c++)) ; __git_dir=\"${words[c]}\" ;;\n \t\t--bare)      __git_dir=\".\" ;;\n \t\t--help) command=\"help\"; break ;;\n-\t\t-c|--work-tree|--namespace) ((c++)) ;;\n+\t\t-c|-C|--work-tree|--namespace) ((c++)) ;;\n \t\t-*) ;;\n \t\t*) command=\"$i\"; break ;;\n \t\tesac\n@@ -2604,13 +2604,13 @@ __git_main ()\n \tfi\n \n \tlocal completion_func=\"_git_${command//-/_}\"\n-\tdeclare -f $completion_func >/dev/null && $completion_func && return\n+\tdeclare -f $completion_func >/dev/null 2>/dev/null && $completion_func && return\n \n \tlocal expansion=$(__git_aliased_command \"$command\")\n \tif [ -n \"$expansion\" ]; then\n \t\twords[1]=$expansion\n \t\tcompletion_func=\"_git_${expansion//-/_}\"\n-\t\tdeclare -f $completion_func >/dev/null && $completion_func\n+\t\tdeclare -f $completion_func >/dev/null 2>/dev/null && $completion_func\n \tfi\n }\n \ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 2ba62fbc17..f5a669918d 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -480,7 +480,12 @@ test_expect_success 'general options plus command' '\n \ttest_completion \"git --namespace=foo check\" \"checkout \" &&\n \ttest_completion \"git --paginate check\" \"checkout \" &&\n \ttest_completion \"git --info-path check\" \"checkout \" &&\n-\ttest_completion \"git --no-replace-objects check\" \"checkout \"\n+\ttest_completion \"git --no-replace-objects check\" \"checkout \" &&\n+\ttest_completion \"git --git-dir some/path check\" \"checkout \" &&\n+\ttest_completion \"git -c conf.var=value check\" \"checkout \" &&\n+\ttest_completion \"git -C some/path check\" \"checkout \" &&\n+\ttest_completion \"git --work-tree some/path check\" \"checkout \" &&\n+\ttest_completion \"git --namespace name/space check\" \"checkout \"\n '\n \n test_expect_success 'git --help completion' '\n-- \n2.6.0.rc2.22.g7128296\n"},{"id":"271208","messageId":"56138BB7.4000404@drmicha.warpmail.net","threadId":"40483","inReplyTo":"1444046535-18316-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] completion: fix completion after 'git -C path'","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-10-06T08:52:07Z","receivedAt":"2015-10-06T08:52:07Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"SZEDER Gábor venit, vidit, dixit 05.10.2015 14:02:\n> The main completion function finds the name of the git command by\n> iterating through all the words on the command line in search for the\n> first non-option-looking word.  As it is not aware of 'git -C's\n> mandatory path argument, if the '-C path' option is present, 'path' will\n> be the first such word and it will be mistaken for a git command.  This\n> breaks the completion script in various ways:\n> \n>  - If 'path' happens to match one of the commands supported by the\n>    completion script, then its options will be offered.\n> \n>  - If 'path' doesn't match a supported command and doesn't contain any\n>    characters not allowed in Bash identifier names, then the completion\n>    script does basically nothing and lets Bash to fall back to filename\n>    completion.\n> \n>  - Otherwise, if 'path' contains such unallowed characters, then it\n>    leads to a more or less ugly error in the middle of the command line.\n>    The standard '/' directory separator is such a character, and it\n>    happens to trigger one of the uglier errors:\n> \n>      $ git -C some/path <TAB>sh.exe\": declare: `_git_some/path': not a valid identifier\n>      error: invalid key: alias.some/path\n> \n> Fix this by skipping 'git -C's mandatory path argument while iterating\n> over the words on the command line.  Extend the relevant test with this\n> case and, while at it, with cases that needed similar treatment in the\n> past ('--git-dir', '-c', '--work-tree' and '--namespace').\n> Additionally, shut up standard error of the 'declare' commands looking\n> for the associated completion function and of the 'git config' query for\n> the aliased command, so if git learns a new option with a mandatory\n> argument in the future, then at least the command line will not be\n> utterly disrupted by those error messages.\n> \n> Note, that this change merely fixes the breakage related to 'git -C\n> path', but the completion script will not take it into account as it\n> does '--git-dir path'.\n\nI don't understand the \"as it does\" part. Do you mean that the\ncompletion script does '--git-dir path', or that git does it?\n\nIn any case, \"git -C path ...\" is more like \"cd path && git ...\". That\nis, completion should take it into account at least when determining\nGIT_DIR (though -C does not specifiy the git-dir directly), and possibly\nalso for completion of untracked files. Otherwise, it's going by the\nwrong repo (unless path is a subdir of cwd).\n\nMichael\n"},{"id":"271211","messageId":"20151006121224.Horde.eFfzROzl5PskJn4vxQYePQ6@webmail.informatik.kit.edu","threadId":"40483","inReplyTo":"56138BB7.4000404@drmicha.warpmail.net","subject":"Re: [PATCH] completion: fix completion after 'git -C path'","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-10-06T10:12:24Z","receivedAt":"2015-10-06T10:12:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Michael J Gruber <git@drmicha.warpmail.net>:\n\n> SZEDER Gábor venit, vidit, dixit 05.10.2015 14:02:\n>> The main completion function finds the name of the git command by\n>> iterating through all the words on the command line in search for the\n>> first non-option-looking word.  As it is not aware of 'git -C's\n>> mandatory path argument, if the '-C path' option is present, 'path' will\n>> be the first such word and it will be mistaken for a git command.  This\n>> breaks the completion script in various ways:\n>>\n>>  - If 'path' happens to match one of the commands supported by the\n>>    completion script, then its options will be offered.\n>>\n>>  - If 'path' doesn't match a supported command and doesn't contain any\n>>    characters not allowed in Bash identifier names, then the completion\n>>    script does basically nothing and lets Bash to fall back to filename\n>>    completion.\n>>\n>>  - Otherwise, if 'path' contains such unallowed characters, then it\n>>    leads to a more or less ugly error in the middle of the command line.\n>>    The standard '/' directory separator is such a character, and it\n>>    happens to trigger one of the uglier errors:\n>>\n>>      $ git -C some/path <TAB>sh.exe\": declare: `_git_some/path':  \n>> not a valid identifier\n>>      error: invalid key: alias.some/path\n>>\n>> Fix this by skipping 'git -C's mandatory path argument while iterating\n>> over the words on the command line.  Extend the relevant test with this\n>> case and, while at it, with cases that needed similar treatment in the\n>> past ('--git-dir', '-c', '--work-tree' and '--namespace').\n>> Additionally, shut up standard error of the 'declare' commands looking\n>> for the associated completion function and of the 'git config' query for\n>> the aliased command, so if git learns a new option with a mandatory\n>> argument in the future, then at least the command line will not be\n>> utterly disrupted by those error messages.\n>>\n>> Note, that this change merely fixes the breakage related to 'git -C\n>> path', but the completion script will not take it into account as it\n>> does '--git-dir path'.\n>\n> I don't understand the \"as it does\" part. Do you mean that the\n> completion script does '--git-dir path', or that git does it?\n\nWhen the user specifies '--git-dir path' on the command line the\ncompletion script respects that (most of the time, I noticed a few\nmissing spots), i.e.\n\n   git --git-dir /somewhere/else/.git log <TAB>\n\ngives you all the refs from the specified repository, which is good.\nHowever, that's not the case with '-C /somewhere/else', as it will lead to\nthe error mentioned in the commit message on current git, or will be\nignored with this patch.\n\n> In any case, \"git -C path ...\" is more like \"cd path && git ...\". That\n> is, completion should take it into account at least when determining\n> GIT_DIR (though -C does not specifiy the git-dir directly), and possibly\n> also for completion of untracked files. Otherwise, it's going by the\n> wrong repo (unless path is a subdir of cwd).\n\nI agree, it should, but it doesn't.  That will be the next step in some\nfuture patches.\n\n\nGábor\n"}]}