[BUG] _less incorrectly completes -" and -#

[BUG] _less incorrectly completes -" and -#

[Roman Perepelitsa]

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

Completing `less -` offers `-"` as a candidate. Accepting it literally
inserts `-"`. I think it should offer `-\"` and insert the same.

My naive patch (posted below) fixes this issue but not completely.
Unexpected behavior with the patch:

  - Completing `less -\" x -` doesn't list any flags.
  - Completing `less --quote=x -` lists `-\"`.

I was unable to fix these.

Roman.

P.S.

`-#` has the same issue.

---

[-- Attachment #2: patch-less.txt --]
[-- Type: text/plain, Size: 875 bytes --]

diff --git a/Completion/Unix/Command/_less b/Completion/Unix/Command/_less
index cb71314a6..cdb3c0e56 100644
--- a/Completion/Unix/Command/_less
+++ b/Completion/Unix/Command/_less
@@ -80,7 +80,7 @@ _arguments -S -s -A "[-+]*"  \
   '--no-keypad[disable use of keypad terminal init string]' \
   '(-y --max-forw-scroll)'{-y,--max-forw-scroll}'[specify forward scroll limit]' \
   '(-z --window)'{-z+,--window=}'[specify scrolling window size]:lines' \
-  '(-\" --quotes)'{-\"+,--quotes=}'[change quoting character]:quoting characters' \
+  '(-\" --quotes)'{-\\\\\"+,--quotes=}'[change quoting character]:quoting characters' \
   '(-~ --tilde)'{-~,--tilde}"[don't display tildes after end of file]" \
   '(-\# --shift)'{-\#+,--shift=}"[specify amount to move when scrolling horizontally]:number" \
   '--follow-name[the F command changes file if the input file is renamed]' \

Re: [BUG] _less incorrectly completes -" and -#

[Oliver Kiddle]

Roman Perepelitsa wrote:
> Completing `less -` offers `-"` as a candidate. Accepting it literally
> inserts `-"`. I think it should offer `-\"` and insert the same.
>
> My naive patch (posted below) fixes this issue but not completely.

It also breaks them for the case where the user has specified an initial
single or double quote.

The real problem is that from inside _arguments, the -Q option gets
passed to compadd. Removing it is a fairly easy change. But I wish I
knew why it was used in the first place. Maybe we should just do that
and worry later about fixing any problems it creates afterwards.

It really seems ugly to be quoting any options in the _arguments
specifications other than those that are special to _arguments itself (=
: [ and +)

Oliver

Re: [BUG] _less incorrectly completes -" and -#

[Bart Schaefer]

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

On Fri, Oct 2, 2020 at 7:47 AM Oliver Kiddle <opk@zsh.org> wrote:

>
> The real problem is that from inside _arguments, the -Q option gets
> passed to compadd. Removing it is a fairly easy change. But I wish I
> knew why it was used in the first place.


The first use of -Q in _arguments happened sometime between workers/7450
and workers/10309, which (unfortunately for quick back-tracing) were all
slurped in as a single commit by PWS when we first migrated to git.  At
that point Sven was still doing 99% of the edits to completion and his
entries in ChangeLog are a bit terse.

[-- Attachment #2: Type: text/html, Size: 878 bytes --]

Re: [BUG] _less incorrectly completes -" and -#

[Oliver Kiddle]

This is digging up another fairly old _arguments issue that I meant to
deal with before:

On 2 Oct 2020, I wrote:
> Roman Perepelitsa wrote:
> > Completing `less -` offers `-"` as a candidate. Accepting it literally
> > inserts `-"`. I think it should offer `-\"` and insert the same.

> The real problem is that from inside _arguments, the -Q option gets
> passed to compadd. Removing it is a fairly easy change. But I wish I
> knew why it was used in the first place. Maybe we should just do that
> and worry later about fixing any problems it creates afterwards.

The following patch does exactly this. _arguments has quite a few lines
that do compadd -Q - "${PREFIX}${SUFFIX}" which I've left alone. They
seem to be special case handling, possibly for _arguments' -w option.
It'd be nice to establish test cases that exercise them but the only
path I could contrive to use one of them did not result in what I'd take
to be intended behaviour (_arguments -s '-a=-' and -a<tab> giving a
space suffix).

Otherwise, this simple approach appears to work well in my testing.

One issue that isn't new is that completion after '- doesn't list
options for a completion that lacks rest-arguments. The _arguments C
code seems to have to deal with the command-line in quoted form. There
used to be a layer of unquoting in the _arguments C code that got
removed in 27218 but restoring it doesn't help anyway.

Most of the patch is completion functions that hard-coded now
superfluous quotes on options. The use of quotes within exclusion lists
was never useful.

Oliver

diff --git a/Completion/Base/Utility/_arguments b/Completion/Base/Utility/_arguments
index 3f1b39304..69e7ab235 100644
--- a/Completion/Base/Utility/_arguments
+++ b/Completion/Base/Utility/_arguments
@@ -513,8 +513,8 @@ if (( $# )) && comparguments -i "$autod" "$singopt[@]" "$@"; then
 	    tmp2=( "${PREFIX}${(@M)^${(@)${(@)tmp1%%:*}#[-+]}:#?}" )
 
             _describe -O option \
-                      tmp1 tmp2 -Q -S '' -- \
-		      tmp3 -Q
+                      tmp1 tmp2 -S '' -- \
+                      tmp3
 
             [[ -n "$optarg" && "$single" = next && nm -eq $compstate[nmatches] ]] &&
                 _all_labels options expl option \
@@ -525,9 +525,9 @@ if (( $# )) && comparguments -i "$autod" "$singopt[@]" "$@"; then
         else
           next+=( "$odirect[@]" )
           _describe -O option \
-                    next -Q -M "$matcher" -- \
-                    direct -QS '' -M "$matcher" -- \
-                    equal -QqS= -M "$matcher"
+                    next -M "$matcher" -- \
+                    direct -S '' -M "$matcher" -- \
+                    equal -qS= -M "$matcher"
         fi
 	PREFIX="$prevpre"
 	IPREFIX="$previpre"
diff --git a/Completion/Darwin/Command/_qtplay b/Completion/Darwin/Command/_qtplay
index 39a7c6de2..839efee83 100644
--- a/Completion/Darwin/Command/_qtplay
+++ b/Completion/Darwin/Command/_qtplay
@@ -15,6 +15,6 @@ _arguments -S \
   '-t[specify update time]:update time (seconds)' \
   '-T[kill time]:ticks' \
   '-V[volume]:percentage of normal volume' \
-  '(-)'{-?,--help,-h}'[display help information]' \
+  '(-)'{-\?,--help,-h}'[display help information]' \
   '(-)*:quicktime file:_files'
 
diff --git a/Completion/Unix/Command/_less b/Completion/Unix/Command/_less
index 0b474b991..ae912a633 100644
--- a/Completion/Unix/Command/_less
+++ b/Completion/Unix/Command/_less
@@ -80,9 +80,9 @@ _arguments -S -s -A "[-+]*"  \
   '--no-keypad[disable use of keypad terminal init string]' \
   '(-y --max-forw-scroll)'{-y,--max-forw-scroll}'[specify forward scroll limit]' \
   '(-z --window)'{-z+,--window=}'[specify scrolling window size]:lines' \
-  '(-\" --quotes)'{-\"+,--quotes=}'[change quoting character]:quoting characters' \
+  '(-" --quotes)'{-\"+,--quotes=}'[change quoting character]:quoting characters' \
   '(-~ --tilde)'{-~,--tilde}"[don't display tildes after end of file]" \
-  '(-\# --shift)'{-\#+,--shift=}"[specify amount to move when scrolling horizontally]:number" \
+  '(-# --shift)'{-\#+,--shift=}"[specify amount to move when scrolling horizontally]:number" \
   '--file-size[automatically determine the size of the input file]' \
   '--incsearch[search file as each pattern character is typed in]' \
   '--line-num-width=[set the width of line number field]:width [7]' \
diff --git a/Completion/Unix/Command/_nm b/Completion/Unix/Command/_nm
index 79f537ac6..888f1ef87 100644
--- a/Completion/Unix/Command/_nm
+++ b/Completion/Unix/Command/_nm
@@ -53,7 +53,7 @@ if _pick_variant -r variant binutils=GNU elftoolchain=elftoolchain elfutils=elfu
 	'(-C)--demangle[decode symbol names]'
 	'(--format -P)-f+[specify output format]:format:(bsd sysv posix)'
 	'(- *)--usage[give a short usage message]'
-	'(- *)-\\?[display help information]'
+	'(- *)-?[display help information]'
       )
     ;;
     binutils)
diff --git a/Completion/Unix/Command/_php b/Completion/Unix/Command/_php
index c4c4ab3e2..9a8f519b1 100644
--- a/Completion/Unix/Command/_php
+++ b/Completion/Unix/Command/_php
@@ -93,7 +93,7 @@ _php() {
     + '(hv)' # Help/version options; kept separate by convention
     '(- 1 *)'{-h,--help}'[display help information]'
     '(- 1 *)'{-v,--version}'[display version information]'
-    '!(- 1 *)'{-\?,-\\\?,--usage}
+    '!(- 1 *)'{-\?,--usage}
 
     + '(im)' # Info/module options (exclusive with everything but -c/-n)
     '(fi mc pb pf rf rn sc sv *)'{-i,--info}'[display configuration information (phpinfo())]'
diff --git a/Completion/Unix/Command/_strings b/Completion/Unix/Command/_strings
index af95af52f..685daa286 100644
--- a/Completion/Unix/Command/_strings
+++ b/Completion/Unix/Command/_strings
@@ -45,7 +45,7 @@ if _pick_variant -r variant binutils=GNU elftoolchain=elftoolchain elfutils=elfu
     elfutils)
       args+=(
 	'(- *)--usage[display a short usage message]'
-	'(- *)-\\?[display help information]'
+	'(- *)-?[display help information]'
       )
     ;;
   esac
diff --git a/Completion/Unix/Command/_zip b/Completion/Unix/Command/_zip
index bc9aab1a5..cfa51be36 100644
--- a/Completion/Unix/Command/_zip
+++ b/Completion/Unix/Command/_zip
@@ -115,7 +115,7 @@ case $service in
       '(-U)-UU[ignore any Unicode fields]' \
       '-W[modify pattern matching so only ** matches /]' \
       '-\:[allow extraction outside of extraction base directory]' \
-      '-\\\^[allow control characters in extracted entries]' \
+      '-^[allow control characters in extracted entries]' \
       '-i[include the following names]:*-*:pattern' \
       '-x[exclude the following names]:*-*:pattern' \
       "(-p -f -u -l -t -z -n -o -j -C -X -q -qq -a -aa -v -L -M)1:zip file:_files -g '(#i)*.(zip|xpi|[ejw]ar)(-.)'" \
diff --git a/Completion/X/Command/_gnome-gv b/Completion/X/Command/_gnome-gv
index 25de6fadf..b1b66e2a4 100644
--- a/Completion/X/Command/_gnome-gv
+++ b/Completion/X/Command/_gnome-gv
@@ -1,6 +1,6 @@
 #compdef gnome-gv ggv
 
 _arguments \
-  '(--help)-\\?[help]' \
+  '(--help)-?[help]' \
   '(--windows)-w[number of empty windows]:number:' \
   '*:file: _pspdf -z' --

Re: [BUG] _less incorrectly completes -" and -#

[Jun. T]

This is the minor problem I mentioned in the previous post (49765).

> 2021/10/23 9:26, Oliver Kiddle <opk@zsh.org> wrote:
> 
> This is digging up another fairly old _arguments issue that I meant to
> deal with before:
> 
> On 2 Oct 2020, I wrote:
>> Roman Perepelitsa wrote:
>>> Completing `less -` offers `-"` as a candidate. Accepting it literally
>>> inserts `-"`. I think it should offer `-\"` and insert the same.

The patch (commit 4e9d00) has solved this problem, but a few problems remain.

zsh[1]% less -\"<TAB><TAB>
The 1st TAB inserts a space, and the 2nd TAB offers list of input files,
but what is expected is a message "quoting characters".

zsh[2]% less -\"cc -<TAB>
this still offers --quote and -"

zsh[3]% less -\" cc -<TAB>
this gives "No match for: 'file'"

Since ca_parse_line() (computil.c:2009) does not remove the escape from the -\"
on the command line (see line 2080 and below), it seems it is not recognized as
a known option.
If I quote the -\"+ in the spec as '-\"+'

'(-" --quotes)'{'-\"+',--quotes=}'[change quoting character]:quoting characters'

then the above problems seem to be "mostly" solved
(in case [1], for the 1st TAB we also get 
-"  -- change quoting character
in addition to the correct message).

But with this spec:

zsh[4]% less --quote=cc -<TAB>
this still offers -"

It seems we also need to escape the " in the exclusion list:

'(-\" --quotes)'{'-\"+',--quotes=}'[change quoting character]:quoting characters'

but is this a good way of fixing these problems?


(this patch is against the current master, not including the patch in 49765).

diff --git a/Completion/Unix/Command/_less b/Completion/Unix/Command/_less
index ae912a633..7ba665069 100644
--- a/Completion/Unix/Command/_less
+++ b/Completion/Unix/Command/_less
@@ -80,9 +80,9 @@ _arguments -S -s -A "[-+]*"  \
   '--no-keypad[disable use of keypad terminal init string]' \
   '(-y --max-forw-scroll)'{-y,--max-forw-scroll}'[specify forward scroll limit]' \
   '(-z --window)'{-z+,--window=}'[specify scrolling window size]:lines' \
-  '(-" --quotes)'{-\"+,--quotes=}'[change quoting character]:quoting characters' \
+  '(-\" --quotes)'{'-\"+',--quotes=}'[change quoting character]:quoting characters' \
   '(-~ --tilde)'{-~,--tilde}"[don't display tildes after end of file]" \
-  '(-# --shift)'{-\#+,--shift=}"[specify amount to move when scrolling horizontally]:number" \
+  '(-\# --shift)'{'-\#+',--shift=}"[specify amount to move when scrolling horizontally]:number" \
   '--file-size[automatically determine the size of the input file]' \
   '--incsearch[search file as each pattern character is typed in]' \
   '--line-num-width=[set the width of line number field]:width [7]' \