Re: [PATCH/RFC] completion: complete refs in multiple steps

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jan 28, 2019, 14:38 UTC
Message-ID: <20190128143828.GJ6702@szeder.dev>
In-Reply-To: <20190128094155.2424-1-pclouds@gmail.com>

On Mon, Jan 28, 2019 at 04:41:55PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 6 quoted lines

> This is in the same spirit of f22f682695 (completion: complete general
> config vars in two steps - 2018-05-27). Instead of considering all full
> refs as completion candidates, it completes one "path" component at a
> time, e.g.
> 
>     $ git switch-branch -d j<TAB>

  $ git switch-branch
  git: 'switch-branch' is not a git command. See 'git --help'.

Please use only existing Git commands in the examples.

Show 19 quoted lines

>     jch/            junio-gpg-pub
> 
>     $ git switch-branch -d jch/<TAB>
>     Display all 154 possibilities? (y or n)
>     jch/ab/            jch/fc/
>     ....
> 
>     $ git switch-branch -d jch/nd/<TAB>
>     jch/nd/attr-pathspec-fix
>     jch/nd/attr-pathspec-in-tree-walk
>     ...
> 
> For refs organized in multiple levels like this (and I've seen refs in 4
> levels), especially when there a lot of refs, incremental completion
> this way makes it easier to get to what you want.
> 
> The cost of course is more complicated completion and also slower on
> systems with slow process creation. So maybe there will be a switch to
> turn this on or off?

Oh, no. After spending quite some effort to eliminate most of the processes and big shell loops from the ref completion code path, I find this patch simply terrible ;)

Show 11 quoted lines

> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 499e56f83d..d74ee79866 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -742,6 +742,17 @@ __git_refs ()
>  	esac
>  }
>  
> +__git_collapse_refs ()
> +{
> +	local regex="$(echo "$1" | sed 's/[^/]\+/[^\/]*/g')"

Surely this could be done using only shell builtins.

> +	case "$regex" in
> +		'') regex='[^\/]*';;

Should this really escape that '/'? The 'sed' below would escape it anyway, so it would be escaped twice. And the line below doesn't escape it.

> +		*/) regex="${regex}[^/]*";;
> +	esac
> +	regex="$(echo "$regex" | sed 's/\//\\\//g')"

And this one, too.

Show 13 quoted lines

> +	sed -ne "s/\\($regex\\/\\?\\).*/\\1/p"
> +}
> +
>  # Completes refs, short and long, local and remote, symbolic and pseudo.
>  #
>  # Usage: __git_complete_refs [<option>]...
> @@ -769,7 +780,7 @@ __git_complete_refs ()
>  		shift
>  	done
>  
> -	__gitcomp_direct "$(__git_refs "$remote" "$track" "$pfx" "$cur_" "$sfx")"
> +	__gitcomp_direct "$(__git_refs "$remote" "$track" "$pfx" "$cur_" "$sfx" | __git_collapse_refs "$cur_")"
>  }

In general I think it would be much better to rely more on 'git for-each-ref' to do the heavy lifting, extending it with new format specifiers/options as necessary.

'%(refname:rstrip=-<N>)' already comes somewhat close to what we would need for full ref completion (i.e. 'refs/b<TAB>' to complete 'refs/bisec/bad'), we only have to figure out how many "ref path components" to show based on the number of path components in the current word to be completed. Alas, it won't add the trailing '/' for "ref directories".

For "regular" refs completion we would need to combine 'rstrip=-<N>' with 'lstrip=2' to remove the 'refs/(heads|tags|remotes)/' prefix, but ref-filter doesn't support things like that, it allows only one format option. And, of course, the lack of trailing '/' is an issue in this case as well.

So perhaps a new format option like '%(refname:path-components=3,2)' to show two ref path components starting with the third with a trailing '/' appended if necessary, e.g. to turn 'refs/remotes/jch/nd/attr-pathspec-fix' into 'jch/nd/'.

Note that we also have __git_head() and __git_tags() to complete only branches and only tags.