Re: [PATCH v2 1/1] completion: load completion file for external subcommand

["Florian Gamböck" <mail@floga.de>]

On 2018-04-23 17:12, SZEDER Gábor wrote:
> On Thu, Apr 19, 2018 at 9:07 PM, Florian Gamböck <mail@floga.de> 
> wrote:
>> On 2018-04-18 21:51, SZEDER Gábor wrote:
>>> Would it be possible to use _xfunc() instead to plug that hole?  It 
>>> seems the be tricky, because that function not only sources but also 
>>> _calls_ the completion function.
>>
>> But isn't this exactly what we want?
>
> No, that's definitely not what we want.
>
> The bash-completion project can get away with it, because they only 
> use their _xfunc() to source a file they themselves ship and to call a 
> completion function they know that that file contains.
>
> We, however, would like to load a file that might not exist and to 
> call a function that might not be defined.  Git has a lot of plumbing 
> commands with neither a corresponding _git_plumbing_cmd() completion 
> function in our completion script nor a corresponding 
> 'git-plumbing-cmd' file that could be sourced dynamically to provide 
> that function.  The same applies to any 'git-foo' command in the 
> user's $PATH (the user's own scripts or various git-related tools, 
> e.g. Git LFS).
>
> So if someone tries e.g. 'git diff-index <TAB>' to complete files in 
> the current directory, then it would result in an error message like
>
>  _git_diff_index: command not found
>
> Furthermore, earlier versions of _xfunc(), I think until the 
> introduction  of __load_completion(), tried to source the file given 
> as parameter without checking its existence beforehand, so on whatever 
> LTS I happen to be currently using I would also get an error like
>
>  bash: /usr/share/bash-completion/completions/git-diff-index: No such 
>  file or directory
>
> Finally, since all this is running in the user's interactive shell, 
> Bash will even run the 'command_not_found_handle', if it's set (and 
> most Linux distros do set it in their default configuration (OK, maybe 
> not most, but at least some of the more popular do)), which may or may 
> not have any suggestions, but at the very least it takes quite a while 
> to scan through its database.

You're right, this could be a problem.

Then how about the following patch? This is one of my very first 
iterations of this patch, before even sending it to the mailing list. 
Actually this is similar to what the original _xfunc did, plus existence 
checking and minus premature function calling. In the directory of the 
git completion script, if there is an appropriate script for the current 
subcommand, if it is a regular file (or a valid symlink) and if it is 
readable, then source it and test the existence of the completion 
function again. Likewise for a possible alias.

-- >8 --
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2615,10 +2615,21 @@ __git_main ()
 	local completion_func="_git_${command//-/_}"
 	declare -f $completion_func >/dev/null && $completion_func && return
 
+	local completion_dir="$(dirname ${BASH_SOURCE[0]})"
+	local completion_file="$completion_dir/git-$command"
+	[ -f "$completion_file" -a -r "$completion_file" ] && . "$completion_file"
+	declare -f $completion_func >/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 && return
+
+		completion_file="$completion_dir/git-$expansion"
+		[ -f "$completion_file" -a -r "$completion_file" ] &&
+			. "$completion_file"
 		declare -f $completion_func >/dev/null && $completion_func
 	fi
 }