zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: "Miroslav Koškár" <mk@mkoskar.com>
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH] _git: Improve handling of aliases
Date: Wed, 24 Jun 2020 11:56:30 +0000	[thread overview]
Message-ID: <20200624115630.GA25495@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <7f7706bd28646cf47d77a0e1b5f89cff40ed9ffe.1592995874.git.mk@mkoskar.com>

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

  reply	other threads:[~2020-06-24 11:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 11:12 Miroslav Koškár
2020-06-24 11:56 ` Daniel Shahaf [this message]
2020-06-26 10:17   ` Miroslav Koškár
2020-06-26 17:00     ` Daniel Shahaf
2020-06-27  6:12       ` [PATCH 0/4] " Miroslav Koškár
2020-06-27  6:12         ` [PATCH 1/4] _git: Remove hanging whitespaces Miroslav Koškár
2020-06-27  6:12         ` [PATCH 2/4] _git: Fix insufficiently quoted pattern Miroslav Koškár
2020-06-27  6:12         ` [PATCH 3/4] _git: Fix handling of aliases Miroslav Koškár
2020-06-27  6:12         ` [PATCH 4/4] _git: Don't shadow global aliases Miroslav Koškár
2020-06-27  9:04       ` _git: Improve handling of aliases Miroslav Koškár
2020-06-27 20:39         ` Daniel Shahaf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200624115630.GA25495@tarpaulin.shahaf.local2 \
    --to=d.s@daniel.shahaf.name \
    --cc=mk@mkoskar.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).