Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Tue, Feb 14, 2017 at 10:24 PM,  <cornelius.weig@tngtech.com> wrote:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Git-checkout completes words starting with '--' as options and other
> words as refs. Even after specifying a ref, further words not starting
> with '--' are completed as refs, which is invalid for git-checkout.

Refs completion is never attempted for words after the disambiguating
double-dash.

Even when refs completion is attempted, if it is unsuccessful, i.e.
there is no ref that matches the current word to be completed, then
Bash falls back to standard filename completion.  No refs match
'./<TAB>'.

Furthermore, Bash performs filename completion on Alt-/ independently
from any completion function.

Granted, none of these will limit to only modified files...  But that
might be a good thing, see below.

> This commit ensures that after specifying a ref, further non-option
> words are completed as paths. Four cases are considered:
>
>  - If the word contains a ':', do not treat it as reference and use
>    regular revlist completion.
>  - If no ref is found on the command line, complete non-options as refs
>    as before.
>  - If the ref is HEAD or @, complete only with modified files because
>    checking out unmodified files is a noop.

Here you use "modified" in the 'ls-files --modified' sense, but that
doesn't include modifications already staged in the index, see below.

>    This case also applies if no ref is given, but '--' is present.
>  - If a ref other than HEAD or @ is found, offer only valid paths from
>    that revision.
>
> Note that one corner-case is not covered by the current implementation:
> if a refname contains a ':' and is followed by '--' the completion would
> not recognize the valid refname.

I'm not sure what you mean here.  Refnames can't contain ':'.

> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> ---
>  contrib/completion/git-completion.bash | 39 +++++++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 4ab119d..df46f62 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1068,7 +1068,7 @@ _git_bundle ()
>
>  _git_checkout ()
>  {
> -       __git_has_doubledash && return
> +       local i c=2 ref="" seen_double_dash=""
>
>         case "$cur" in
>         --conflict=*)
> @@ -1081,13 +1081,36 @@ _git_checkout ()
>                         "
>                 ;;
>         *)
> -               # check if --track, --no-track, or --no-guess was specified
> -               # if so, disable DWIM mode
> -               local flags="--track --no-track --no-guess" track=1
> -               if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
> -                       track=''
> -               fi
> -               __gitcomp_nl "$(__git_refs '' $track)"
> +               while [ $c -lt $cword ]; do
> +                       i="${words[c]}"
> +                       case "$i" in
> +                       --) seen_double_dash=1 ;;
> +                       -*|?*:*) ;;
> +                       *) ref="$i"; break ;;

I haven't tried it, but this would trigger on e.g. 'git checkout -b
new-feature <TAB>', wouldn't it?

> +                       esac
> +                       ((c++))
> +               done
> +
> +               case "$ref,$seen_double_dash,$cur" in
> +               ,,*:*)
> +                   __git_complete_revlist_file
> +                   ;;
> +               ,,*)
> +                       # check for --track, --no-track, or --no-guess
> +                       # if so, disable DWIM mode
> +                       local flags="--track --no-track --no-guess" track=1
> +                       if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
> +                               track=''
> +                       fi
> +                       __gitcomp_nl "$(__git_refs '' $track)"
> +                       ;;
> +               ,1,*|@,*|HEAD,*)
> +                       __git_complete_index_file "--modified"

What about

  $ echo "unintentional change" >>tracked-file && git add -u
  $ git rm important-file
  $ git checkout HEAD <TAB>

?  It seems it will offer neither 'tracked-file' nor 'important-file',
but I think it should offer both.


We have __git_complete_index_file() for a while now, but only use it
for commands that accept only --options and filenames, e.g. 'add',
'clean', 'rm'.  This would be the first case when we would use it for
a command that accept both refs and filenames.  Perhaps similar corner
cases and the easy ways to trigger filename completion are why no one
thought it's worth it.

> +                       ;;
> +               *)
> +                       __git_complete_tree_file "$ref" "$cur"

Well, here you could go all-in, and say that this should complete only
files that are different from the version in $ref, because checking
out files that are still the same is a noop :)

> +                       ;;
> +               esac
>                 ;;
>         esac
>  }
> --
> 2.10.2
>