zsh-workers
 help / color / Atom feed
* [PATCH] Merge ignore and deduplication patterns in _sequence
@ 2020-03-09 11:32 oxiedi
  2020-05-21 12:43 ` Oliver Kiddle
  0 siblings, 1 reply; 3+ messages in thread
From: oxiedi @ 2020-03-09 11:32 UTC (permalink / raw)
  To: zsh-workers

First, it fixes the following bug:

% zsh -f
% autoload -Uz compinit
% compinit -D
% compdef '_arguments ": :_sequence compadd - 1 2 3"' seq
% zstyle ':completion:*:*:seq:*:*' ignored-patterns 2
% seq <Tab>
_sequence:40: command not found: -F

Second, it allows a user to ignore matches using the ignored-patterns style.

diff --git a/Completion/Base/Utility/_sequence b/Completion/Base/Utility/_sequence
index c1ff32184..cb79960f2 100644
--- a/Completion/Base/Utility/_sequence
+++ b/Completion/Base/Utility/_sequence
@@ -8,10 +8,11 @@
 # -d     : duplicate values allowed
 
 local curcontext="$curcontext" nm="$compstate[nmatches]" pre qsep nosep minus
-local -a opts sep num pref suf cont end uniq dedup
+local ign_pats
+local -a opts sep num pref suf cont end ign uniq dedup
 
 zparseopts -D -a opts s:=sep n:=num p:=pref i:=pref P:=pref I:=suf S:=suf \
-    q=suf r:=suf R:=suf C:=cont d=uniq M+: J+: V+: 1 2 o+: X+: x+:
+    q=suf r:=suf R:=suf C:=cont F:=ign d=uniq M+: J+: V+: 1 2 o+: X+: x+:
 (( $#cont )) && curcontext="${curcontext%:*}:$cont[2]"
 (( $#sep )) || sep[2]=,
 
@@ -20,12 +21,22 @@ if (( $+suf[(r)-S] )); then
   (( $#end )) && compset -S ${end}\* && suf=() && nosep=1
 fi
 
+ign_pats=$ign[2]
+if [[ $ign_pats == \(*\) ]]; then
+  ign=( ${=ign_pats[2,-2]} )
+elif [[ -n $ign_pats ]]; then
+  ign=( ${(P)ign_pats} )
+else
+  ign=()
+fi
+
 qsep="${sep[2]}"
 compquote -p qsep
 if (( ! $#uniq )); then
   (( $+pref[(r)-P] )) && pre="${(q)pref[pref[(i)-P]+1]}"
   dedup=( "${(@)${(@ps.$qsep.)PREFIX#$pre}[1,-2]}" "${(@)${(@ps.$qsep.)SUFFIX}[2,-1]}" )
   [[ -n $compstate[quoting] ]] || dedup=( ${(Q)dedup} )
+  ign+=( "$dedup[@]" )
 fi
 
 if (( $#num )) && compset -P $(( num[2] - 1 )) \*${(q)qsep}; then
@@ -37,4 +48,4 @@ else
 fi
 
 (( minus = argv[(ib:2:)-] ))
-"${(@)argv[1,minus-1]}" "$opts[@]" -F dedup "$pref[@]" "$suf[@]" "${(@)argv[minus+1,-1]}"
+"${(@)argv[1,minus-1]}" "$opts[@]" -F ign "$pref[@]" "$suf[@]" "${(@)argv[minus+1,-1]}"
diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index 51f604bcf..c616e606e 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -216,6 +216,57 @@ F:regression test workers/31611
 >NO:{3pm}
 >NO:{10pm}
 
+  comptesteval "_tst() { _arguments ':desc:_sequence -d compadd - 1 2 3' }"
+  comptesteval "zstyle ':completion:*:tst:*' ignored-patterns 2"
+  comptest $'tst 1,\t'
+0:ignore matches in _sequence via the ignored-patterns style (without deduplication)
+>line: {tst 1,}{}
+>DESCRIPTION:{desc}
+>NO:{1}
+>NO:{3}
+
+  comptesteval "_tst() { _arguments ':desc:_sequence compadd - 1 2 3 4' }"
+  comptest $'tst 1,\t'
+  comptesteval "zstyle -d ':completion:*:tst:*' ignored-patterns"
+0:ignore matches in _sequence via the ignored-patterns style (with deduplication)
+>line: {tst 1,}{}
+>DESCRIPTION:{desc}
+>NO:{3}
+>NO:{4}
+
  comptest $'a=() b=(\t'
 0:multiple envarrays
 >line: {a=() b=(}{}

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

* Re: [PATCH] Merge ignore and deduplication patterns in _sequence
  2020-03-09 11:32 [PATCH] Merge ignore and deduplication patterns in _sequence oxiedi
@ 2020-05-21 12:43 ` Oliver Kiddle
  2020-05-22 19:02   ` oxiedi
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Kiddle @ 2020-05-21 12:43 UTC (permalink / raw)
  To: oxiedi; +Cc: zsh-workers

On 9 Mar, oxiedi@yandex.ru wrote:
> First, it fixes the following bug:
>
> % zsh -f
> % autoload -Uz compinit
> % compinit -D
> % compdef '_arguments ": :_sequence compadd - 1 2 3"' seq
> % zstyle ':completion:*:*:seq:*:*' ignored-patterns 2
> % seq <Tab>
> _sequence:40: command not found: -F
>
> Second, it allows a user to ignore matches using the ignored-patterns style.

Sorry for taking so long to review this patch. Currently, _sequence
ignores a -F that is passed to it and in general it is good for helper
functions to accept as many of them as possible. As I'll explain, I think
the omission was intentional in this case. I'd be interested in what
other people think because the change is perhaps more useful in actual
practice.

A compadd option passed to _sequence would normally apply to the whole
list rather than individual elements. Using -S as an example:
  _foo() {
    _sequence -S/ -s, -n2 compadd - 1 2 3
  }
  foo 1,2<tab>  - completes the / suffix, we have a complete list (-n2)
So / is a suffix of the whole list rather than of individual elements.

So if _sequence is passed a -F option then arguably the pattern should 
apply to the whole list: -F '(1,2)' would exclude the list 1,2. However,
there's no simple way to implement that and it doesn't seem especially
useful. And in practical terms, while we normally have _sequence wrap
_wanted rather than the other way around, sometimes it is the
inverse as in the _arguments example.

I'm inclined to apply the change. Anyone else have a view?

There is one caveat which is that for the array passed to -F, we rely on
the dynamic scoping of local variables in zsh so if _sequence calls a
function with a local variable name clash then things break. ign is very
commonly used in completion functions. dedup was rather less common.
Perhaps we should call it _sequence_ign or something. I'm not worried
about nested use of _sequence because ignored patterns from an outer
_sequence are unlikely to be especially helpful.

Oliver

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

* Re: [PATCH] Merge ignore and deduplication patterns in _sequence
  2020-05-21 12:43 ` Oliver Kiddle
@ 2020-05-22 19:02   ` oxiedi
  0 siblings, 0 replies; 3+ messages in thread
From: oxiedi @ 2020-05-22 19:02 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: zsh-workers


[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

Thank you for the review.

I believe there is one more issue. In the following example:

  compdef '_sequence _wanted tag expl desc compadd - 1 2 3' foo

the `tag` is not known to _sequence yet. So a user can't ignore matches
using e.g.

  zstyle ':completion:*:foo:*:tag' ignored-patterns 2 '2,*' '*,2,*' '*,2'

which is a bit inconsistent. I'd have fixed it by merging _sequence
patterns with _comp_ignore in _description, but that would be too much
code changes for a small gain.

For now, I suggest to ignore -F and its argument in order to fix the
`command not found: -F` error (patch attached).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ignore--F-in-_sequence.patch --]
[-- Type: text/x-diff; name="ignore--F-in-_sequence.patch", Size: 1378 bytes --]

diff --git a/Completion/Base/Utility/_sequence b/Completion/Base/Utility/_sequence
index c1ff32184..1a87c1753 100644
--- a/Completion/Base/Utility/_sequence
+++ b/Completion/Base/Utility/_sequence
@@ -8,10 +8,10 @@
 # -d     : duplicate values allowed
 
 local curcontext="$curcontext" nm="$compstate[nmatches]" pre qsep nosep minus
-local -a opts sep num pref suf cont end uniq dedup
+local -a opts sep num pref suf cont end uniq dedup garbage
 
 zparseopts -D -a opts s:=sep n:=num p:=pref i:=pref P:=pref I:=suf S:=suf \
-    q=suf r:=suf R:=suf C:=cont d=uniq M+: J+: V+: 1 2 o+: X+: x+:
+    q=suf r:=suf R:=suf C:=cont F:=garbage d=uniq M+: J+: V+: 1 2 o+: X+: x+:
 (( $#cont )) && curcontext="${curcontext%:*}:$cont[2]"
 (( $#sep )) || sep[2]=,
 
diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index 51f604bcf..a00103946 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -216,6 +216,16 @@ F:regression test workers/31611
 >NO:{3pm}
 >NO:{10pm}
 
+  comptesteval "_tst() { _arguments ':desc:_sequence compadd - 1 2 3' }"
+  comptesteval "zstyle ':completion:*:tst:*' ignored-patterns 2"
+  comptest $'tst 1,\t'
+  comptesteval "zstyle -d ':completion:*:tst:*' ignored-patterns"
+0:-F doesn't break _sequence
+>line: {tst 1,}{}
+>DESCRIPTION:{desc}
+>NO:{2}
+>NO:{3}
+
  comptest $'a=() b=(\t'
 0:multiple envarrays
 >line: {a=() b=(}{}

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 11:32 [PATCH] Merge ignore and deduplication patterns in _sequence oxiedi
2020-05-21 12:43 ` Oliver Kiddle
2020-05-22 19:02   ` oxiedi

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git