Re: _git-reset doesn't complete newly staged files

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Jun T. wrote on Mon, Mar 07, 2016 at 12:16:54 +0900:
> Are there any comments to my patch? Especially from those
> who know git better.
> If there is nothing obviously wrong, I'll push it.

Sorry for not replying earlier: I had this flagged for attention but
haven't spoken up sooner since I expected I wouldn't have time to reply
to responses.

More below.

Jun T. wrote on Thu, Mar 03, 2016 at 14:07:12 +0900:
> In the case of
> 
> % git reset HEAD <TAB>
> 
> the patch below tries to offer only staged files (including
> newly staged files which have never been committed).
> A new function __git_staged_files() ignores unmerged (conflicting)
> files, but I'm not sure this is correct or not.
> 
> In the case of
> 
> % git reset commit <TAB>    # commit is not HEAD
> 
> I have no idea what to do, so I just added __git_ignore_line
> to the original code. 
> 
> 
> 
> diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
> index b7eaf2e..7a459f1 100644
> --- a/Completion/Unix/Command/_git
> +++ b/Completion/Unix/Command/_git
> @@ -1377,7 +1377,11 @@ _git-reset () {
>        if [[ -n $line[1] ]] && __git_is_committish $line[1]; then
>          commit=$line[1]
>        fi

This should actually call __git_is_treeish, not __git_is_committish.

(That's a preëxisting problem: it precedes your patch.)

> -      __git_tree_files ${PREFIX:-.} $commit && ret=0
> +      if [[ $commit == HEAD ]]; then

As you say, special-casing HEAD this way is not wrong, but shouldn't be
needed: "HEAD" is a treeish and should be treated like any other
treeish.

I think you're looking for «git diff-index --cached -z --name-only
$treeish».  That gives the names of files that differ between the index
and $treeish.  (I wrote «$treeish» in pseudocode, but the real code's
local parameter «$commit» contains exactly the right value.)

I'm not sure how to handle the syntax 'git reset foo' where 'foo' is
both a file in the index and a treeish (e.g., a tag name).

Incidentally, it would be nice to have a docstring for __git_ignore_line:

    diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
    index b7eaf2e..8043bb9 100644
    --- a/Completion/Unix/Command/_git
    +++ b/Completion/Unix/Command/_git
    @@ -5034,6 +5034,16 @@ __git_describe_commit () {
     
     # Completion Wrappers
     
    +# '__git_ignore_line $callee "${callee_args[@]}" "${callee_compadd_args[@]}"'
    +# invokes '$callee "${callee_args[@]}" "${callee_compadd_args[@]}"' with
    +# callee_compadd_args modified to exclude positional parameters to the completed
    +# command from being completed.  This causes 'git add foo <TAB>' not to offer
    +# 'foo' again.
    +#
    +# Note: This function can't be used to wrap bare 'compadd' calls that use a '--'
    +# argument terminator.  It can wrap functions of the form
    +#     f() { shift $N; compadd "$@" -a - mymatches }
    +# .
     (( $+functions[__git_ignore_line] )) ||
     __git_ignore_line () {
       declare -a ignored

Why does __git_ignore_line escape some characters with backslashes
before adding them to $ignored?

Thanks,

Daniel

> +	__git_ignore_line __git_staged_files && ret=0
> +      else
> +	__git_ignore_line __git_tree_files ${PREFIX:-.} $commit && ret=0
> +      fi
>        ;;
>    esac
>  
> @@ -6131,6 +6135,29 @@ __git_tree_files () {
>    _wanted files expl 'tree file' _multi_parts -f $compadd_opts -- / tree_files
>  }
>  
> +(( $+functions[__git_staged_files] )) ||
> +__git_staged_files () {
> +  local -a slist staged_files
> +  local item expl i
> +
> +  slist=(${(0)"$(_call_program staged-files git status -z -uno 2>/dev/null)"})
> +  __git_command_successful $pipestatus || return 1
> +
> +  for (( i = 1; i <= $#slist; ++i )) do
> +    item=$slist[i]
> +    if [[ $item == (DD|AA|U|?U)* ]]; then
> +      continue	  #XXX skip unmerged files
> +    elif [[ $item == R* ]]; then
> +      staged_files+=( $item[4,-1] $slist[++i] )
> +    elif [[ $item == [ACDM]* ]]; then
> +      staged_files+=( $item[4,-1] )
> +    fi
> +  done
> +  staged_files=( ${(0)"$(__git_files_relative ${(pj:\0:)staged_files})"} )
> +
> +  _wanted staged-files expl 'staged file' compadd "$@" -a - staged_files
> +}
> +
>  # Repository Argument Types
>  
>  (( $+functions[__git_remote_repositories] )) ||
> 
> 
> 
> 
>