git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCHv2 14/21] completion: fix completion after 'git -C <path>'
Date: Fri,  3 Feb 2017 03:48:22 +0100	[thread overview]
Message-ID: <20170203024829.8071-15-szeder.dev@gmail.com> (raw)
In-Reply-To: <20170203024829.8071-1-szeder.dev@gmail.com>

The main completion function finds the name of the git command by
iterating through all the words on the command line in search for the
first non-option-looking word.  As it is not aware of 'git -C's
mandatory path argument, if the '-C <path>' option is present, 'path'
will be the first such word and it will be mistaken for a git command.
This breaks completion in various ways:

 - If 'path' happens to match one of the commands supported by the
   completion script, then options of that command will be offered.

 - If 'path' doesn't match a supported command and doesn't contain any
   characters not allowed in Bash identifier names, then the
   completion script does basically nothing and Bash in turn falls
   back to filename completion for all subsequent words.

 - Otherwise, if 'path' does contain such an unallowed character, then
   it leads to a more or less ugly error message in the middle of the
   command line.  The standard '/' directory separator is such a
   character, and it happens to trigger one of the uglier errors:

     $ git -C some/path <TAB>sh.exe": declare: `_git_some/path': not a valid identifier
     error: invalid key: alias.some/path

Fix this by skipping 'git -C's mandatory path argument while iterating
over the words on the command line.  Extend the relevant test with
this case and, while at it, with cases that needed similar treatment
in the past ('--git-dir', '-c', '--work-tree' and '--namespace').

Additionally, silence the standard error of the 'declare' builtins
looking for the completion function associated with the git command
and of the 'git config' query for the aliased command.  So if git ever
learns a new option with a mandatory argument in the future, then,
though the completion script will again misbehave, at least the
command line will not be utterly disrupted by those error messages.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash | 8 ++++----
 t/t9902-completion.sh                  | 7 ++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7d25b33b8..ac5eb9ced 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -816,7 +816,7 @@ __git_aliases ()
 __git_aliased_command ()
 {
 	local word cmdline=$(git --git-dir="$(__gitdir)" \
-		config --get "alias.$1")
+		config --get "alias.$1" 2>/dev/null)
 	for word in $cmdline; do
 		case "$word" in
 		\!gitk|gitk)
@@ -2800,7 +2800,7 @@ __git_main ()
 		--git-dir)   ((c++)) ; __git_dir="${words[c]}" ;;
 		--bare)      __git_dir="." ;;
 		--help) command="help"; break ;;
-		-c|--work-tree|--namespace) ((c++)) ;;
+		-c|-C|--work-tree|--namespace) ((c++)) ;;
 		-*) ;;
 		*) command="$i"; break ;;
 		esac
@@ -2844,13 +2844,13 @@ __git_main ()
 	fi
 
 	local completion_func="_git_${command//-/_}"
-	declare -f $completion_func >/dev/null && $completion_func && return
+	declare -f $completion_func >/dev/null 2>/dev/null && $completion_func && return
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then
 		words[1]=$expansion
 		completion_func="_git_${expansion//-/_}"
-		declare -f $completion_func >/dev/null && $completion_func
+		declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
 	fi
 }
 
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 500505dca..be2ed8704 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -755,7 +755,12 @@ test_expect_success 'general options plus command' '
 	test_completion "git --namespace=foo check" "checkout " &&
 	test_completion "git --paginate check" "checkout " &&
 	test_completion "git --info-path check" "checkout " &&
-	test_completion "git --no-replace-objects check" "checkout "
+	test_completion "git --no-replace-objects check" "checkout " &&
+	test_completion "git --git-dir some/path check" "checkout " &&
+	test_completion "git -c conf.var=value check" "checkout " &&
+	test_completion "git -C some/path check" "checkout " &&
+	test_completion "git --work-tree some/path check" "checkout " &&
+	test_completion "git --namespace name/space check" "checkout "
 '
 
 test_expect_success 'git --help completion' '
-- 
2.11.0.555.g967c1bcb3


  parent reply	other threads:[~2017-02-03  2:50 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  2:48 [PATCHv2 00/21] completion: various __gitdir()-related improvements SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 01/21] completion: improve __git_refs()'s in-code documentation SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 02/21] completion tests: don't add test cruft to the test repository SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 03/21] completion tests: make the $cur variable local to the test helper functions SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 04/21] completion tests: consolidate getting path of current working directory SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 05/21] completion tests: check __gitdir()'s output in the error cases SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 06/21] completion tests: add tests for the __git_refs() helper function SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 07/21] completion: ensure that the repository path given on the command line exists SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 08/21] completion: fix most spots not respecting 'git --git-dir=<path>' SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 09/21] completion: respect 'git --git-dir=<path>' when listing remote refs SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 10/21] completion: list refs from remote when remote's name matches a directory SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 11/21] completion: don't list 'HEAD' when trying refs completion outside of a repo SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 12/21] completion: list short refs from a remote given as a URL SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 13/21] completion: don't offer commands when 'git --opt' needs an argument SZEDER Gábor
2017-02-03  2:48 ` SZEDER Gábor [this message]
2017-02-03  2:48 ` [PATCHv2 15/21] rev-parse: add '--absolute-git-dir' option SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 16/21] completion: respect 'git -C <path>' SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 17/21] completion: don't use __gitdir() for git commands SZEDER Gábor
2017-02-13 19:20   ` [PATCH] completion: restore removed line continuating backslash SZEDER Gábor
2017-02-13 20:45     ` Junio C Hamano
2017-02-15  1:35       ` SZEDER Gábor
2017-02-15  2:41         ` Junio C Hamano
2017-02-15 13:06           ` SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 18/21] completion: consolidate silencing errors from git commands SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 19/21] completion: don't guard git executions with __gitdir() SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 20/21] completion: extract repository discovery from __gitdir() SZEDER Gábor
2017-02-03  2:48 ` [PATCHv2 21/21] completion: cache the path to the repository SZEDER Gábor
2017-02-06 21:29 ` [PATCHv2 00/21] completion: various __gitdir()-related improvements Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170203024829.8071-15-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).