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

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

[Nguyễn Thái Ngọc Duy]

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

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')"
+	case "$regex" in
+		'') regex='[^\/]*';;
+		*/) regex="${regex}[^/]*";;
+	esac
+	regex="$(echo "$regex" | sed 's/\//\\\//g')"
+	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_")"
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
-- 
2.20.1.560.g70ca8b83ee

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

[SZEDER Gábor]

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.

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

[Jeff King]

On Mon, Jan 28, 2019 at 03:38:28PM +0100, SZEDER Gábor wrote:

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

FWIW, that was my first thought, too.

> '%(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".

I think it also makes it hard to do one thing which (I think) people
would want: if there is a single deep ref, complete the whole thing.
E.g., given:

  $ git for-each-ref --refname='%(refname)'
  refs/heads/foo/bar
  refs/heads/foo/baz
  refs/heads/another/deep/one

we'd ideally complete "fo" to "foo/" and "ano" to "another/deep/one",
rather than making the user tab through each level.

Doing that requires actually understanding that the refs are in a list,
and not formatting each one independently. So I kind of wonder if it
would be easier to simply have a completion mode in ref-filter.

-Peff

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

[Jeff King]

On Mon, Jan 28, 2019 at 12:27:07PM -0500, Jeff King wrote:

> Doing that requires actually understanding that the refs are in a list,
> and not formatting each one independently. So I kind of wonder if it
> would be easier to simply have a completion mode in ref-filter.

Er, I meant "a completion mode in for-each-ref", though I think it
largely works out to be the same thing.

-Peff

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

[Duy Nguyen]

On Tue, Jan 29, 2019 at 12:27 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jan 28, 2019 at 03:38:28PM +0100, SZEDER Gábor wrote:
>
> > > -   __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.
>
> FWIW, that was my first thought, too.

I was more concerned whether it's a good idea to begin with. But it
sounds like you two both think it's good otherwise would have
objected.

> > '%(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".
>
> I think it also makes it hard to do one thing which (I think) people
> would want: if there is a single deep ref, complete the whole thing.
> E.g., given:
>
>   $ git for-each-ref --refname='%(refname)'
>   refs/heads/foo/bar
>   refs/heads/foo/baz
>   refs/heads/another/deep/one
>
> we'd ideally complete "fo" to "foo/" and "ano" to "another/deep/one",
> rather than making the user tab through each level.

Ah ha, like github sometimes show nested submodule paths. Just one
small modification, when doing "refs/heads/<tab>" I would just show

refs/heads/foo/
refs/heads/another/

not refs/heads/another/deep/one to save space. But when you do
"refs/heads/a<tab>" then you get "refs/heads/another/deep/one"
immediately.

> Doing that requires actually understanding that the refs are in a list,
> and not formatting each one independently. So I kind of wonder if it
> would be easier to simply have a completion mode in for-each-mode.

That also allows more complicated logic. I think sometimes completion
code gets it wrong (I think it's often the case with rev/path
ambiguation, but maybe dwim stuff too). And we already have all this
logic in C.
-- 
Duy

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

[Jeff King]

On Tue, Jan 29, 2019 at 07:43:45AM +0700, Duy Nguyen wrote:

> > > 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.
> >
> > FWIW, that was my first thought, too.
> 
> I was more concerned whether it's a good idea to begin with. But it
> sounds like you two both think it's good otherwise would have
> objected.

Heh. I do not have any real objection, but I don't think I'd really know
until I used it for a while. It may be a matter of preference, in which
case we might want $GIT_COMPLETION_REF_COMPONENTS to enable/disable it
(I don't have an opinion on what the default should be).

> >   $ git for-each-ref --refname='%(refname)'
> >   refs/heads/foo/bar
> >   refs/heads/foo/baz
> >   refs/heads/another/deep/one
> >
> > we'd ideally complete "fo" to "foo/" and "ano" to "another/deep/one",
> > rather than making the user tab through each level.
> 
> Ah ha, like github sometimes show nested submodule paths. Just one
> small modification, when doing "refs/heads/<tab>" I would just show
> 
> refs/heads/foo/
> refs/heads/another/
> 
> not refs/heads/another/deep/one to save space. But when you do
> "refs/heads/a<tab>" then you get "refs/heads/another/deep/one"
> immediately.

Yeah. It's still only one entry either way (by definition), but it's
going to be much longer than all of the others. Again, I think I'd have
to see it in practice to decide how much I cared either way.

> > Doing that requires actually understanding that the refs are in a list,
> > and not formatting each one independently. So I kind of wonder if it
> > would be easier to simply have a completion mode in for-each-mode.
> 
> That also allows more complicated logic. I think sometimes completion
> code gets it wrong (I think it's often the case with rev/path
> ambiguation, but maybe dwim stuff too). And we already have all this
> logic in C.

Yeah, agreed.

-Peff

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

[SZEDER Gábor]

On Tue, Jan 29, 2019 at 07:43:45AM +0700, Duy Nguyen wrote:
> On Tue, Jan 29, 2019 at 12:27 AM Jeff King <peff@peff.net> wrote:

> > > 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.
> >
> > FWIW, that was my first thought, too.
> 
> I was more concerned whether it's a good idea to begin with. But it
> sounds like you two both think it's good otherwise would have
> objected.

Well, I objected to this RFC implementation, but on purpose refrained
from expressing an opinion about whether this is good or bad idea,
because I didn't even want to try to guess what others might prefer :)

And as far as I can tell it doesn't affect my usage (in general I
don't have multi-level hierarchies under refs/heads/, so I mostly
complete a single ref path component, and when on occasion I do
complete a full ref, then I tend to know what I want, and even if I
don't, I only need the list of possibilities only at the last ref path
component).

> > > '%(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".
> >
> > I think it also makes it hard to do one thing which (I think) people
> > would want: if there is a single deep ref, complete the whole thing.
> > E.g., given:
> >
> >   $ git for-each-ref --refname='%(refname)'
> >   refs/heads/foo/bar
> >   refs/heads/foo/baz
> >   refs/heads/another/deep/one
> >
> > we'd ideally complete "fo" to "foo/" and "ano" to "another/deep/one",
> > rather than making the user tab through each level.
> 
> Ah ha, like github sometimes show nested submodule paths. Just one
> small modification, when doing "refs/heads/<tab>" I would just show
> 
> refs/heads/foo/
> refs/heads/another/
> 
> not refs/heads/another/deep/one to save space. But when you do
> "refs/heads/a<tab>" then you get "refs/heads/another/deep/one"
> immediately.

But this would be inconsistent with how "regular" path completion
works, e.g. if you have 'unique-dir/only-one-dir/only-one-file' and do
e.g. 'cat u<TAB>', then Bash will only offer 'unique-dir/', not the
whole path to the file in the subdir.  Ok, I know, those are real
paths not refs, they just look alike, because both happen to use '/'
as separator.

I can't asses whether people, in general, would prefer this behavior
or would rather be surprised or bothered by the inconsistency.