Re: [PATCH] _git: Improve handling of aliases

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

A couple of quick comments below.  (This is not a full review.)

Miroslav Koškár wrote on Wed, Jun 24, 2020 at 13:12:59 +0200:
> ---
> This handles all of the following aliases:
> 
> # simple
> lg = log --format=one --graph --color
> 
> # nested
> lga = lg --all
> 
> # nested with preceding git options
> push-upstream = -c push.default=upstream pushv
> pushv = push -v
> 
> # simple shell
> l = !git -P lg -5
> 
> # shell pipeline (completion of last command)
> conf = !git config -l --local | sort

*nod*

> +++ b/Completion/Unix/Command/_git
> @@ -8082,35 +8082,8 @@ __git_color_attributes () {
>  
>  # Now, for the main drive...
>  _git() {
> -  if (( CURRENT > 2 )); then
> -    local -a aliases
> -    local -A git_aliases
> -    local a k v
> -    local endopt='!(-)--end-of-options'
> -    aliases=(${(0)"$(_call_program aliases git config -z --get-regexp '\^alias\.')"})

This (preëxisting) line is wrong.  The arguments to _call_program are as
to «eval»: joined with spaces and then one layer of quoting removed.
That means the backslash that protects the dot doesn't make it through
to git.  (If that weren't so, the backslash before the caret would have
caused the regexp to never match.)

You moved the surrounding block code further down the function
unchanged.  I'd recommend to split the patch into two parts: one that
just moves the code without changing it, and one that adds new
functionality.  That'll make it much easier to review.

> +        local git_alias=git_aliases[\$words[1]]
> +        if (( ${(P)+git_alias} && !$+commands[git-$words[1]] && !$+functions[_git-$words[1]] )); then
> +          git_alias=${(P)git_alias}
> +          local len=$#words
> +          if [[ $git_alias = \!* ]]; then
> +            local i
> +            local -a git_alias_tail
> +            for i in "${(z)git_alias##\!}"; do
> +              git_alias_tail+=("$i")
> +              [[ $i = \| ]] && git_alias_tail=()

Would it be possible to add support to everything that could be used
there, in addition to pipes?  I think you just need to call whatever «sh
-c <TAB>» does, i.e., _cmdstring.  That would add support for comments
and for a bunch of things other than «|» that can be followed by the
start of a simple command (for example, the tokens and reserved words
in https://github.com/zsh-users/zsh-syntax-highlighting/blob/91d2eeaf23c47341e8dc7ad66dbf85e38c2674de/highlighters/main/main-highlighter.zsh#L391-L416).

Adding support for comments specifically could also be done by using
the ${(Z)} flag, but I don't know how common that is.  If «foo = bar # baz»
lines in .gitconfig pass the «#» sign through to sh, then support for
comments should probably be added, one way or another.

Cheers,

Daniel

> +            done
> +            words=("$git_alias_tail[@]" "${words[@]:1}")
> +          else
> +            words=("${allwords[@]:0:-$#words}" "${(z)git_alias}" "${words[@]:1}")
> +          fi
> +          (( CURRENT += $#words - len ))
> +          _normal -P && ret=0
> +        else
> +          curcontext=${curcontext%:*:*}:git-$words[1]:
> +          (( $+opt_args[--git-dir] )) && local -x GIT_DIR=${(Q)${~opt_args[--git-dir]}}
> +          (( $+opt_args[--work-tree] )) && local -x GIT_WORK_TREE=${(Q)${~opt_args[--work-tree]}}
> +          if ! _call_function ret _git-$words[1]; then
> +            if zstyle -T :completion:$curcontext: use-fallback; then
> +              _default && ret=0
> +            else
> +              _message "unknown sub-command: $words[1]"
> +            fi
> +          fi
>          fi
>          ;;
>        (configuration)
> -- 
> 2.27.0
> 
>