From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Mon, 28 Jan 2019 14:38:28 GMT
Subject: Re: [PATCH/RFC] completion: complete refs in multiple steps
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:
> 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.

>     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 ;)

> 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.

> +	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.