zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] _git: Improve handling of aliases
@ 2020-06-24 11:12 Miroslav Koškár
  2020-06-24 11:56 ` Daniel Shahaf
  0 siblings, 1 reply; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-24 11:12 UTC (permalink / raw)
  To: zsh-workers

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

PS: Even though unrelated I kept first 2 hunks that strip hanging
whitespaces.

 Completion/Unix/Command/_git | 83 ++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 42 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 9eeff6a..cff6750 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -2346,7 +2346,7 @@ __git_config_value () {
 # Helper to _git-config().  May be called by other functions, too, provided
 # that The caller has set $line, $state, and $opt_args as _git-config() would
 # set them:
-# 
+#
 # - set $line[1] to the option name being completed (even if completing an
 #   option value).
 # - set $opt_args to git-config(1) options, as set by _arguments in
@@ -2916,7 +2916,7 @@ __git_config_option-or-value () {
     for key in $git_present_options ; do
       if (( ${+git_options[(r)(#i)${(b)key}:*]} )); then
         # $key is already in git_options
-        continue 
+        continue
       elif (( ${+sections_that_permit_arbitrary_subsection_names[(r)${(b)key%%.*}]} )); then
         if [[ $key == *.*.* ]]; then
           # If $key isn't an instance of a known foo.*.bar:baz $git_options entry...
@@ -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\.')"})
-    for a in ${aliases}; do
-        k="${${a/$'\n'*}/alias.}"
-        v="${a#*$'\n'}"
-        git_aliases[$k]="$v"
-    done
-
-    if (( $+git_aliases[$words[2]] && !$+commands[git-$words[2]] && !$+functions[_git-$words[2]] )); then
-      local -a tmpwords expalias
-      expalias=(${(z)git_aliases[$words[2]]})
-      tmpwords=(${words[1]} ${expalias})
-      if [[ -n "${words[3,-1]}" ]] ; then
-          tmpwords+=(${words[3,-1]})
-      fi
-      [[ -n ${words[$CURRENT]} ]] || tmpwords+=('')
-      (( CURRENT += ${#expalias} - 1 ))
-      words=("${tmpwords[@]}")
-      unset tmpwords expalias
-    fi
-
-    unset git_aliases aliases
-  fi
-
   integer ret=1
+  local -a allwords=("${words[@]}")
 
   if [[ $service == git ]]; then
     local curcontext=$curcontext state line
@@ -8143,18 +8116,44 @@ _git() {
         _git_commands && ret=0
         ;;
       (option-or-argument)
-        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 [[ $words[1] = \!* ]]; then
-	    words[1]=${words[1]##\!}
-	    _normal && ret=0
-	  elif zstyle -T :completion:$curcontext: use-fallback; then
-	    _default && ret=0
-	  else
-	    _message "unknown sub-command: $words[1]"
-	  fi
+        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\.')"})
+        for a in $aliases; do
+          k=${${a/$'\n'*}/alias.}
+          v=${a#*$'\n'}
+          git_aliases[$k]=$v
+        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}
+          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=()
+            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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] _git: Improve handling of aliases
  2020-06-24 11:12 [PATCH] _git: Improve handling of aliases Miroslav Koškár
@ 2020-06-24 11:56 ` Daniel Shahaf
  2020-06-26 10:17   ` Miroslav Koškár
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Shahaf @ 2020-06-24 11:56 UTC (permalink / raw)
  To: Miroslav Koškár; +Cc: zsh-workers

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _git: Improve handling of aliases
  2020-06-24 11:56 ` Daniel Shahaf
@ 2020-06-26 10:17   ` Miroslav Koškár
  2020-06-26 17:00     ` Daniel Shahaf
  0 siblings, 1 reply; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-26 10:17 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _git: Improve handling of aliases
  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  9:04       ` _git: Improve handling of aliases Miroslav Koškár
  0 siblings, 2 replies; 11+ messages in thread
From: Daniel Shahaf @ 2020-06-26 17:00 UTC (permalink / raw)
  To: Miroslav Koškár; +Cc: zsh-workers

[To the list: There's an unrelated question about the
${foo[(I)${(~j)}]} syntax at the end.]

Miroslav Koškár wrote on Fri, 26 Jun 2020 12:17 +0200:
> Hi Daniel,
> 
> thanks for your feedback!
> 

You're welcome.

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

Sorry, I don't follow.  I'm not sure how your (1) and (2) map to my points.

To clarify, I was asking that you arrange the series to have one commit
that simply moves lines around in the file with as few changes as
possible, and then one (or more) other commits that make functional
changes.  The idea is to make it easier to review the diff — not just
not before it's merged, but also later on when reviewing the history.

Perhaps the series should have three commits: your (1), your (2), and
a "move lines around in the file" commit (not necessarily in this order)?

> 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

The caret is insufficiently escaped: it needs to be protected from
EXTENDED_GLOB both on this physical line of code, and later when it's
passed to «eval» inside _call_program.

The dot is insufficiently escaped: the physical line of code will eat
one backslash, «eval» will eat the other, and the dot will not be
backslashed by the time the regexp compiler sees it.

>           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

Normally I'd say that clarity is more important than golf and churn,
but on this instance, it does look better not to have to allocate
mental registers to those used-on-two-lines-and-never-again variables.
Unshadowing the predefined $aliases variable is certainly a bonus.

On the other hand, having git_alias contain values of different types at
different times isn't exactly best practice, but *shrug*.

By the way, I've been meaning to ask you if you could please leave the
whitespace changes out, or confined to a separate commit.

>     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

Sure.  Alternatively, we could continue to retrieve all aliases but
cache that list between calls (see _store_cache; there are examples in
_subversion).

>         * it loses configurability of _call_program, which IMO is
>           rather pointless anyway

I'm not too happy about this.  That configurability is already there.
Presumably it was added for a reason, and there's no reason to break
anybody's proverbial spacebar heating.

Also, it's the house style.

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

Well, yes and no.  The arguments to shell aliases seem to be handled
similarly to arguments to «eval»: joined by spaces and then passed to
system(3).  That means it's valid to pass the entire string in a single
shell word, or to translate any of the spaces in the desired result
string into word breaks at the shell input level, just like «eval hello
world» and «eval 'hello world'» are equivalent.

So, how about completing this the same way «eval -- <TAB>» is completed?
Currently ${_comps[eval]} is _precommand, which just calls _normal, but
that's incomplete¹.  If we write the code to do something along the
lines of «words=(eval -- …); CURRENT=…; _normal», it'll automatically
grow support for completing both variants once such support is added to
«eval»'s completion.²

¹ For example, «eval 'git -c' <TAB>» completes files rather than
  configuration options, because it takes 'git -c' rather than just 'git'
  to be the command word.

² That'd be something along the lines of «words=( ${=:-"${words}"} )»,
  I guess?  Plus adjusting $CURRENT, etc..

> 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}")

As above, I'd prefer to delegate to «eval»'s completion.  Alternatively,
my previous point about supporting comments stands.

I was going to mention the «foo[1,(I)bar]=()» syntax, but it doesn't
work as I expect:

    % a=( foo '&&' bar '||' baz )
    % b=( '||' '&&' ) 
    % print ${a[(I)${(~j.|.)b}]} 
    2
    % 

Why doesn't that print 4?  It does print 4 if $b[1] is set to «'\|\|'».

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 0/4] _git: Improve handling of aliases
  2020-06-26 17:00     ` Daniel Shahaf
@ 2020-06-27  6:12       ` Miroslav Koškár
  2020-06-27  6:12         ` [PATCH 1/4] _git: Remove hanging whitespaces Miroslav Koškár
                           ` (3 more replies)
  2020-06-27  9:04       ` _git: Improve handling of aliases Miroslav Koškár
  1 sibling, 4 replies; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-27  6:12 UTC (permalink / raw)
  To: zsh-workers; +Cc: d.s

Hi Daniel,

I believe I addressed everything in the following patchset up until the
last part of what to do with shell aliases. Since that would be a new
functionality anyway I've decided to keep it out for now (I will reply
to that separately).

I've split this up as you wanted (I hope), please feel free to amend
commit messages as you see fit.

Thanks

Miroslav Koškár (4):
  _git: Remove hanging whitespaces
  _git: Fix insufficiently quoted pattern
  _git: Fix handling of aliases
  _git: Don't shadow global aliases

 Completion/Unix/Command/_git | 73 +++++++++++++++---------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] _git: Remove hanging whitespaces
  2020-06-27  6:12       ` [PATCH 0/4] " Miroslav Koškár
@ 2020-06-27  6:12         ` Miroslav Koškár
  2020-06-27  6:12         ` [PATCH 2/4] _git: Fix insufficiently quoted pattern Miroslav Koškár
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-27  6:12 UTC (permalink / raw)
  To: zsh-workers; +Cc: d.s

---
 Completion/Unix/Command/_git | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 9eeff6a..fc754e7 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -2346,7 +2346,7 @@ __git_config_value () {
 # Helper to _git-config().  May be called by other functions, too, provided
 # that The caller has set $line, $state, and $opt_args as _git-config() would
 # set them:
-# 
+#
 # - set $line[1] to the option name being completed (even if completing an
 #   option value).
 # - set $opt_args to git-config(1) options, as set by _arguments in
@@ -2916,7 +2916,7 @@ __git_config_option-or-value () {
     for key in $git_present_options ; do
       if (( ${+git_options[(r)(#i)${(b)key}:*]} )); then
         # $key is already in git_options
-        continue 
+        continue
       elif (( ${+sections_that_permit_arbitrary_subsection_names[(r)${(b)key%%.*}]} )); then
         if [[ $key == *.*.* ]]; then
           # If $key isn't an instance of a known foo.*.bar:baz $git_options entry...
-- 
2.27.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/4] _git: Fix insufficiently quoted pattern
  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         ` 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
  3 siblings, 0 replies; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-27  6:12 UTC (permalink / raw)
  To: zsh-workers; +Cc: d.s

---
 Completion/Unix/Command/_git | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index fc754e7..602d0d0 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -8087,7 +8087,7 @@ _git() {
     local -A git_aliases
     local a k v
     local endopt='!(-)--end-of-options'
-    aliases=(${(0)"$(_call_program aliases git config -z --get-regexp '\^alias\.')"})
+    aliases=(${(0)"$(_call_program aliases git config -z --get-regexp '\^alias\\.')"})
     for a in ${aliases}; do
         k="${${a/$'\n'*}/alias.}"
         v="${a#*$'\n'}"
-- 
2.27.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/4] _git: Fix handling of aliases
  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         ` Miroslav Koškár
  2020-06-27  6:12         ` [PATCH 4/4] _git: Don't shadow global aliases Miroslav Koškár
  3 siblings, 0 replies; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-27  6:12 UTC (permalink / raw)
  To: zsh-workers; +Cc: d.s

* nested aliases
* aliases preceded with options e.g., -c <name>=<value>
---
 Completion/Unix/Command/_git | 73 ++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 40 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 602d0d0..e2d3d6e 100644
--- a/Completion/Unix/Command/_git
+++ 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\\.')"})
-    for a in ${aliases}; do
-        k="${${a/$'\n'*}/alias.}"
-        v="${a#*$'\n'}"
-        git_aliases[$k]="$v"
-    done
-
-    if (( $+git_aliases[$words[2]] && !$+commands[git-$words[2]] && !$+functions[_git-$words[2]] )); then
-      local -a tmpwords expalias
-      expalias=(${(z)git_aliases[$words[2]]})
-      tmpwords=(${words[1]} ${expalias})
-      if [[ -n "${words[3,-1]}" ]] ; then
-          tmpwords+=(${words[3,-1]})
-      fi
-      [[ -n ${words[$CURRENT]} ]] || tmpwords+=('')
-      (( CURRENT += ${#expalias} - 1 ))
-      words=("${tmpwords[@]}")
-      unset tmpwords expalias
-    fi
-
-    unset git_aliases aliases
-  fi
-
   integer ret=1
+  local -a allwords=("${words[@]}")
 
   if [[ $service == git ]]; then
     local curcontext=$curcontext state line
@@ -8143,18 +8116,38 @@ _git() {
         _git_commands && ret=0
         ;;
       (option-or-argument)
-        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 [[ $words[1] = \!* ]]; then
-	    words[1]=${words[1]##\!}
-	    _normal && ret=0
-	  elif zstyle -T :completion:$curcontext: use-fallback; then
-	    _default && ret=0
-	  else
-	    _message "unknown sub-command: $words[1]"
-	  fi
+        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\\.')"})
+        for a in ${aliases}; do
+            k="${${a/$'\n'*}/alias.}"
+            v="${a#*$'\n'}"
+            git_aliases[$k]="$v"
+        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}
+          local len=$#words
+          if [[ $git_alias = \!* ]]; then
+            words=("${(z)git_alias##\!}" "${words[@]:1}")
+          else
+            words=("${allwords[@]:0:-$#words}" "${(z)git_alias}" "${words[@]:1}")
+          fi
+          (( CURRENT += $#words - len ))
+          _normal && 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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/4] _git: Don't shadow global aliases
  2020-06-27  6:12       ` [PATCH 0/4] " Miroslav Koškár
                           ` (2 preceding siblings ...)
  2020-06-27  6:12         ` [PATCH 3/4] _git: Fix handling of aliases Miroslav Koškár
@ 2020-06-27  6:12         ` Miroslav Koškár
  3 siblings, 0 replies; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-27  6:12 UTC (permalink / raw)
  To: zsh-workers; +Cc: d.s

---
 Completion/Unix/Command/_git | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index e2d3d6e..6f7a462 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -8116,15 +8116,11 @@ _git() {
         _git_commands && ret=0
         ;;
       (option-or-argument)
-        local -a aliases
         local -A git_aliases
-        local a k v
+        local a
         local endopt='!(-)--end-of-options'
-        aliases=(${(0)"$(_call_program aliases git config -z --get-regexp '\^alias\\.')"})
-        for a in ${aliases}; do
-            k="${${a/$'\n'*}/alias.}"
-            v="${a#*$'\n'}"
-            git_aliases[$k]="$v"
+        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
-- 
2.27.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _git: Improve handling of aliases
  2020-06-26 17:00     ` Daniel Shahaf
  2020-06-27  6:12       ` [PATCH 0/4] " Miroslav Koškár
@ 2020-06-27  9:04       ` Miroslav Koškár
  2020-06-27 20:39         ` Daniel Shahaf
  1 sibling, 1 reply; 11+ messages in thread
From: Miroslav Koškár @ 2020-06-27  9:04 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

Hi Daniel,

bellow to things not addressed in my recent patchset.

On Jun 26, Daniel Shahaf wrote:
> Sure.  Alternatively, we could continue to retrieve all aliases but
> cache that list between calls (see _store_cache; there are examples in
> _subversion).

I see, good to know. It seems fast enough to me as is so *shrug*.

> Well, yes and no.  The arguments to shell aliases seem to be handled
> similarly to arguments to «eval»: joined by spaces and then passed to
> system(3).  That means it's valid to pass the entire string in a single
> shell word, or to translate any of the spaces in the desired result
> string into word breaks at the shell input level, just like «eval hello
> world» and «eval 'hello world'» are equivalent.
>
> So, how about completing this the same way «eval -- <TAB>» is completed?
> Currently ${_comps[eval]} is _precommand, which just calls _normal, but
> that's incomplete¹.  If we write the code to do something along the
> lines of «words=(eval -- …); CURRENT=…; _normal», it'll automatically
> grow support for completing both variants once such support is added to
> «eval»'s completion.²
>
> ¹ For example, «eval 'git -c' <TAB>» completes files rather than
>   configuration options, because it takes 'git -c' rather than just 'git'
>   to be the command word.
>
> ² That'd be something along the lines of «words=( ${=:-"${words}"} )»,
>   I guess?  Plus adjusting $CURRENT, etc..

Right, I understand your points generally. I see the similarity to eval,
even though there is an important distinction I think:

    (git alias) test = !echo hello
    $ git test \; world
    hello ; world

So git executes aliases more akin to:

    sh -c 'echo hello "$@"' arg0 \; world

As for doing 'words=(eval ...); _normal', it doesn't work for complex
commands e.g., eval cd dir '&&' ls -<TAB>.

I understand you would like to have that work in future and I agree what
would be cool and would certainly help here. With my limited knowledge
of zsh internals and complexity of completion I'll leave this to others
:). While proposed split on simple command separators might seem crude
it actually works quite fine.

> As above, I'd prefer to delegate to «eval»'s completion.  Alternatively,
> my previous point about supporting comments stands.

Ahhh, so that's (Z+C+) instead of (z), forgot about that.
Well it's not like a common case to use comments there, mind you it's
a bit weird to even use them in gitconfig, managed to make an example:

    test = !echo hello \n \
        "#" let this be a commented line \n \
        echo world

Ok, I'll wait for your comments on the patchset I've sent before and
this too and then either incorporate (Z+C+) with other changes or sent
it separately.

Regards,
Miro


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: _git: Improve handling of aliases
  2020-06-27  9:04       ` _git: Improve handling of aliases Miroslav Koškár
@ 2020-06-27 20:39         ` Daniel Shahaf
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2020-06-27 20:39 UTC (permalink / raw)
  To: Miroslav Koškár; +Cc: zsh-workers

Miroslav Koškár wrote on Sat, 27 Jun 2020 11:04 +0200:
> Hi Daniel,
> 
> bellow to things not addressed in my recent patchset.
> 
> On Jun 26, Daniel Shahaf wrote:
> > Sure.  Alternatively, we could continue to retrieve all aliases but
> > cache that list between calls (see _store_cache; there are examples in
> > _subversion).  
> 
> I see, good to know. It seems fast enough to me as is so *shrug*.
> 
> > Well, yes and no.  The arguments to shell aliases seem to be handled
> > similarly to arguments to «eval»: joined by spaces and then passed to
> > system(3).  That means it's valid to pass the entire string in a single
> > shell word, or to translate any of the spaces in the desired result
> > string into word breaks at the shell input level, just like «eval hello
> > world» and «eval 'hello world'» are equivalent.
> >
> > So, how about completing this the same way «eval -- <TAB>» is completed?
> > Currently ${_comps[eval]} is _precommand, which just calls _normal, but
> > that's incomplete¹.  If we write the code to do something along the
> > lines of «words=(eval -- …); CURRENT=…; _normal», it'll automatically
> > grow support for completing both variants once such support is added to
> > «eval»'s completion.²
> >
> > ¹ For example, «eval 'git -c' <TAB>» completes files rather than
> >   configuration options, because it takes 'git -c' rather than just 'git'
> >   to be the command word.
> >
> > ² That'd be something along the lines of «words=( ${=:-"${words}"} )»,
> >   I guess?  Plus adjusting $CURRENT, etc..  
> 
> Right, I understand your points generally. I see the similarity to eval,
> even though there is an important distinction I think:
> 
>     (git alias) test = !echo hello
>     $ git test \; world
>     hello ; world
> 
> So git executes aliases more akin to:
> 
>     sh -c 'echo hello "$@"' arg0 \; world
> 
> As for doing 'words=(eval ...); _normal', it doesn't work for complex
> commands e.g., eval cd dir '&&' ls -<TAB>.

Okay; that's a bug in «eval»'s completion.

> I understand you would like to have that work in future and I agree what
> would be cool and would certainly help here. With my limited knowledge
> of zsh internals and complexity of completion I'll leave this to others
> :).

The easiest way to get started would be to edit _precommand and add
special-case code for when [[ $service = eval ]].  Before committing
we might break out the code to a separate file, but don't worry about
that at this stage; the roadblock is the "handle spaces and command
separators" logic, not the spinning off a new file.  I imagine the
algorithm would be something like:

1. Join all the words with spaces.
2. Pass through ${(z)} [or ${(Z)} for comments support].
3. Trim all words that are before (after) the command separator before
(after) the cursor.
4. I'm not 100% sure about the next step.  Set IPREFIX and ISUFFIX
correctly and then call _cmdstring?

> While proposed split on simple command separators might seem crude
> it actually works quite fine.

Indeed.

> > As above, I'd prefer to delegate to «eval»'s completion.  Alternatively,
> > my previous point about supporting comments stands.  
> 
> Ahhh, so that's (Z+C+) instead of (z), forgot about that.
> Well it's not like a common case to use comments there, mind you it's
> a bit weird to even use them in gitconfig, managed to make an example:
> 
>     test = !echo hello \n \
>         "#" let this be a commented line \n \
>         echo world
> 

Ah, nice, thanks for figuring that out!  Given it's so awkward, and not
supported by the incumbent code, I'd say it's not a priority to support.

> Ok, I'll wait for your comments on the patchset I've sent before and
> this too and then either incorporate (Z+C+) with other changes or sent
> it separately.

Sure.

I've applied the first two patches of the set (thanks!).  Patch 3/4
looks larger; I'm not sure I'll be able to review it tonight, and
unfortunately I have a busy few days ahead of me.  (-workers@, if
anyone wants to jump in, feel free.)

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-06-27 20:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 11:12 [PATCH] _git: Improve handling of aliases Miroslav Koškár
2020-06-24 11:56 ` Daniel Shahaf
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

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