Re: _git: Improve handling of aliases

[Miroslav Koškár <mk@xxxxxxxxxxx>]

Hi Daniel,

thanks for your feedback!

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

True. Noted.

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

Hmm, ok how about this:

1) fixing up for nested aliases and preceding options
2) basic parsing of shell aliases for completing of last simple command
   (this would be the "new functionality" bit then)

For 1) I'm thinking about this:

    a) keep it as is more/less (fixing the small _call_program issue above):

        ...
        local -A git_aliases
        local a
        for a in ${(0)"$(_call_program aliases git config -z --get-regexp \^alias\\.)"}; do
          git_aliases[${${a/$'\n'*}/alias.}]=${a#*$'\n'}
        done
        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}
        ...

        * I've got rid of "aliases", "k" and "v" to shorten it a bit

    b) much shorter and IMO cleaner:

        ...
        local git_alias
        if git_alias=${"$(git config -z --get alias.$words[1] 2>/dev/null)"/$'\0'*} \
          && (( !$+commands[git-$words[1]] && !$+functions[_git-$words[1]] )); then
        ...

        * basically the difference is that we don't ask for all aliases
          but only for the one we're interested in
        * it loses configurability of _call_program, which IMO is
          rather pointless anyway

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

I agree, I've hastily put only split on '|' there. I've looked into
_cmdstring and I don't believe this will be of any help here as it's
completing single command argument only. It seems to me most sane here
is to complete using _normal on last simple command contained in that
alias. So something like this should work fine:

    local i
    local -a git_alias_tail
    for i in "${(z)git_alias##\!}"; do
      git_alias_tail+=("$i")
      case $i in ';' | $'\n' | '||' | '&&' | '|' | '|&' | '&' | '&!' | '&|') git_alias_tail=() ;; esac
    done
    words=("$git_alias_tail[@]" "${words[@]:1}")

Let me know what you think and I'll re-roll the patch/es.

Regards,
Miro