2 new patches for _surfraw

2 new patches for _surfraw

[Marc Chantreux]

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

hello people,

here are

* one comply to good practices
* one bugfix

any review is warmly welcome.

regards,
marc

[-- Attachment #2: 0001-pluralize-tags-in-_surfraw.patch --]
[-- Type: text/x-diff, Size: 3499 bytes --]

From d6acd1c9bb02d77d5102f7319c327e9aec702e2f Mon Sep 17 00:00:00 2001
From: Marc Chantreux <eiro@phear.org>
Date: Fri, 9 Apr 2021 21:44:54 +0200
Subject: [PATCH 1/2] pluralize tags in _surfraw

---
 Completion/Unix/Command/_surfraw | 44 +++++++++++++++++---------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/Completion/Unix/Command/_surfraw b/Completion/Unix/Command/_surfraw
index 343d275cc..09852475e 100644
--- a/Completion/Unix/Command/_surfraw
+++ b/Completion/Unix/Command/_surfraw
@@ -20,11 +20,11 @@ case $state in
   subcmd)
     args=(
       '-help[display help information]'
-      '*:string:_guard "^-*" "search string"'
+      '*:strings:_guard "^-*" "search string"'
     )
     case "$words[1]" in
       ask|cia|cnn|deblogs|excite|filesearching|foldoc|happypenguin|slashdot|slinuxdoc|sundocs|sunsolve|xxx)
-        _message -e string 'search string'
+        _message -e strings 'search string'
       ;;
       alioth)
         _arguments $args \
@@ -164,7 +164,7 @@ case $state in
           '-doc[view PTS documentation]' && ret=0
       ;;
       debsec)
-        _message -e string 'package name, bug number or CVE ID'
+        _message -e strings 'package name, bug number or CVE ID'
       ;; 
       deja)
         _arguments $args \
@@ -261,9 +261,9 @@ case $state in
           '-head=:display headlines:(on off)' \
           '-grid=:show results in grid:(on off)' \
           '-spell=:spelling tolerance:(off standard force)' \
-          '*:dictionary word:->dictword' && ret=0
+          '*:dictionary word:->words' && ret=0
 
-        [[ "$state" = dictword ]] && _wanted words expl 'dictionary word' \
+        [[ "$state" = words ]] && _wanted words expl 'dictionary word' \
             compadd $(look "${PREFIX}") && ret=0
       ;;
       netbsd|openbsd)
@@ -422,27 +422,31 @@ case $state in
           '-results=-[specify number of results to return]:number' && ret=0
       ;;
       yubnub)
-        _message -e command 'Yubnub Command'
+        _message -e commands 'Yubnub Command'
       ;;
       *)
-        _message -e string 'search string'
+        _message -e strings 'search string'
       ;;
     esac
   ;;
   elvi)
-    local -UT XDG_CONFIG_DIRS xcd
-    # as it starts with a space, the header becomes an empty
-    # string removed by the list expansion
-    _wanted elvi expl elvi compadd \
-        ${${(f)"$(surfraw -elvi)"}%%[[:space:]]*} $(
-          # extract the keys of all bookmarks (should be in surfraw itself)
-          awk '{keys[$1]=1} END {for (k in keys) print k}' \
-          $^xcd/surfraw/bookmarks(Nr) \
-          /etc/xdg/surfraw/bookmarks(Nr) \
-          /etc/surfraw.bookmarks(Nr) \
-          ${XDG_CONFIG_HOME-${HOME?homeless}/.config}/surfraw/bookmarks(Nr) \
-          ${HOME?homeless}/.surfraw.bookmarks(Nr)
-        ) && ret=0
+    local -UT XDG_CONFIG_DIRS xdg_config_dirs
+    local it
+    # list the applets
+    set -- ${${(f)"$(_call_program elvi surfraw -elvi)"}%%[[:space:]]##-- *}
+    shift # the first line is an header: remove it
+    # then list the bookmarks
+    for it in \
+      $^xdg_config_dirs/surfraw/bookmarks(Nr) \
+      /etc/xdg/surfraw/bookmarks(Nr) \
+      /etc/surfraw.bookmarks(Nr) \
+      ${XDG_CONFIG_HOME-$HOME/.config}/surfraw/bookmarks(Nr) \
+      $HOME/.surfraw.bookmarks(Nr)
+    do
+      read -d'\0' it < $it
+      set -- "$@" ${${(f)it}%%[[:space:]]*}
+    done
+    _wanted elvi expl elvi compadd "$@" && ret=0
   ;;
 esac
 
-- 
2.30.2


[-- Attachment #3: 0002-fix-ebay-completion-bug-due-to-tailing-space.patch --]
[-- Type: text/x-diff, Size: 1385 bytes --]

From 210a35e9512d3add59774c3012c4ebc241411e55 Mon Sep 17 00:00:00 2001
From: Marc Chantreux <eiro@phear.org>
Date: Fri, 9 Apr 2021 21:45:40 +0200
Subject: [PATCH 2/2] fix ebay completion bug due to tailing space

also removing all the other extra spaces
---
 Completion/Unix/Command/_surfraw | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Completion/Unix/Command/_surfraw b/Completion/Unix/Command/_surfraw
index 09852475e..81f8d635f 100644
--- a/Completion/Unix/Command/_surfraw
+++ b/Completion/Unix/Command/_surfraw
@@ -165,7 +165,7 @@ case $state in
       ;;
       debsec)
         _message -e strings 'package name, bug number or CVE ID'
-      ;; 
+      ;;
       deja)
         _arguments $args \
           '-results=-:[number of results to return]' \
@@ -183,7 +183,7 @@ case $state in
       ;;
       ebay)
         _arguments $args \
-          '-country=-:country:(com de uk fr)' \ 
+          '-country=-:country:(com de uk fr)' \
           '-results=-:[number of results to return]:number' && ret=0
       ;;
       etym)
@@ -233,7 +233,7 @@ case $state in
 	  '-title[search titles (default)]'
 	  '-author[search authors]'
 	  '-num[search etext numbers]'
-      ;;       
+      ;;
       imdb)
         _arguments $args \
           '-category=-:category:(All Titles MyMovies People Characters Quotes Bios Plots)' && ret=0
-- 
2.30.2

Re: 2 new patches for _surfraw

[Oliver Kiddle]

Marc Chantreux wrote:
> any review is warmly welcome.

This mostly looks all good so there's not much I can say.

> -      '*:string:_guard "^-*" "search string"'
> +      '*:strings:_guard "^-*" "search string"'

Thats not actually a tag there but a description. _arguments uses tags
and groups with names like argument-rest and option-J-1. _guard doesn't
take a tag as a parameter and it replaces any description with the new
one. _message may pick it up the tag from $curtag. I know this is far
from intuitive but in this case, it is fine to just do:

  '*: :_guard "^-*" "search string"'

> -    local -UT XDG_CONFIG_DIRS xcd

This looks like a duplicate of the patch in users/26579 from here on.
That was already applied in ccc7ff9. I don't need a fresh patch without
this unless you're sending one anyway.

Looking at the existing _surfraw completion there are other things that
might be improved on or that I suspect may be broken:

| _arguments -C -A \
|   '-browser=[set browser]:browser:_command_names' \

The -A option to _arguments takes an argument. "-*" is common to match
options, indicating that no more options are allowed after the first
non-option argument. In this case, it is preventing -browser from being
completed. It is also worth verifying if -S would be applicable, i.e. --
terminates options and -s, i.e. -tq is allowed as a short form of -t -q.

Aside from that, it should probably be _command_names -e to limit it the
external commands.

| '-quiet:bool:(yes no)' \

Should this be -quiet= ?
Commands tend to be consistent and = was needed for -browser and
-escape-url-args. For many of the elvi, it is =- which means you have to
use, e.g. -type=soft and not -type soft

|  '(-t -text)'{-t,-text}'[back to the yellow brick road]' 

Are -t and -g mutually exlusive? If so, they could be listed in each
others exclusion list, e.g. '(-t -text -g -graphical)'{....

There may be other possible exclusions, e.g. what options are possible
after -elvi?

|      cite)
|        _arguments \
|          '-results=-:[number of results to return]' \

This doesn't look right. The colon should probably come after the
square brackets. And as the description in square brackets is a
description of the -results option and not of the argument that follows
it, I'd probably add a verb like "specify" or "give" to the beginning.
This occurs more than once in the file.

| '-lines=-[specify maximum lines per message]:lines:(0 5 10 50 100)' \

Perhaps use compadd -o nosort. If any number is allowed it may be better
to leave it blank instead of providing matches.

| '-gg=-[search [Google Groups]:enable:(yes no)' \

There's an extra opening bracket in this.

Oliver

Re: 2 new patches for _surfraw

[Daniel Shahaf]

Oliver Kiddle wrote on Fri, Apr 09, 2021 at 23:53:07 +0200:
> Marc Chantreux wrote:
> | '-lines=-[specify maximum lines per message]:lines:(0 5 10 50 100)' \
> 
> Perhaps use compadd -o nosort. If any number is allowed it may be better
> to leave it blank instead of providing matches.
> 

s/:lines:/:number of lines:/

The `message' part should state the expected data type.

> | '-gg=-[search [Google Groups]:enable:(yes no)' \

This doesn't seem to be quoted from the message you were replying to.

Re: 2 new patches for _surfraw

[Oliver Kiddle]

Daniel Shahaf wrote:
> > | '-gg=-[search [Google Groups]:enable:(yes no)' \
>
> This doesn't seem to be quoted from the message you were replying to.

Indeed. The latter parts (marked with | instead of >) were comments on
aspects of the existing _surfraw function. This was stated but may not
have been especially clear. I hope Marc doesn't regard it as
impertinent that I included that commentary, particularly within
the context of an earlier off-list exchange. I hoped it might be
useful within the wider context of improvements to _surfraw.

Oliver

Re: 2 new patches for _surfraw

[Marc Chantreux]

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

hello,

> This mostly looks all good so there's not much I can say.

yet: i want to read carefully and learn about the completion primitives
before submitting a new version so let's focus on the second patch
for the moment (easier and actually fixing a bug).

i propose 2 versions of the patch. the second one also

* remove the other tailing spaces
* remove some tabs in the indentation
* declare the correct indentation in a vim mode

regards
marc

[-- Attachment #2: 0001-fix-ebay-completion-bug-due-to-tailing-space.patch --]
[-- Type: text/x-diff, Size: 4952 bytes --]

From 6367f7dae6bfe27d6580d68ecc87eece8caa845b Mon Sep 17 00:00:00 2001
From: Marc Chantreux <eiro@phear.org>
Date: Sat, 10 Apr 2021 10:25:02 +0200
Subject: [PATCH] fix ebay completion bug due to tailing space

also
* remove the other tailing spaces
* remove some tabs in the indentation
* declare the correct indentation in a vim mode
---
 Completion/Unix/Command/_surfraw | 61 ++++++++++++++++----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/Completion/Unix/Command/_surfraw b/Completion/Unix/Command/_surfraw
index f945f1ca9..b480e3c28 100644
--- a/Completion/Unix/Command/_surfraw
+++ b/Completion/Unix/Command/_surfraw
@@ -1,4 +1,5 @@
 #compdef surfraw sr
+# vim: et ts=2
 
 local curcontext="$curcontext" expl state line args ret=1
 
@@ -64,10 +65,10 @@ case $state in
           '*:search string' && ret=0
       ;;
       ctan)
-	_arguments $args \
-	'-name[search by filename]' \
-	'-desc[search descriptions (default)]'\
-	'-doc[search documentation]'
+    _arguments $args \
+    '-name[search by filename]' \
+    '-desc[search descriptions (default)]'\
+    '-doc[search documentation]'
       ;;
       currency)
         _arguments \
@@ -165,7 +166,7 @@ case $state in
       ;;
       debsec)
         _message -e string 'package name, bug number or CVE ID'
-      ;; 
+      ;;
       deja)
         _arguments $args \
           '-results=-:[number of results to return]' \
@@ -183,7 +184,7 @@ case $state in
       ;;
       ebay)
         _arguments $args \
-          '-country=-:country:(com de uk fr)' \ 
+          '-country=-:country:(com de uk fr)' \
           '-results=-:[number of results to return]:number' && ret=0
       ;;
       etym)
@@ -220,7 +221,7 @@ case $state in
         _deb_packages avail && ret=0
       ;;
       fsfdir)
-	_arguments $args
+    _arguments $args
       ;;
       google)
         _arguments $args \
@@ -229,19 +230,19 @@ case $state in
           '-search=-:topic:(bsd linux mac unclesam)' && ret=0
       ;;
       gutenberg)
-	_arguments $args \
-	  '-title[search titles (default)]'
-	  '-author[search authors]'
-	  '-num[search etext numbers]'
-      ;;       
+    _arguments $args \
+      '-title[search titles (default)]'
+      '-author[search authors]'
+      '-num[search etext numbers]'
+      ;;
       imdb)
         _arguments $args \
           '-category=-:category:(All Titles MyMovies People Characters Quotes Bios Plots)' && ret=0
       ;;
       ixquick)
-	_arguments $args \
-	  '-search=-:search type:(web pics)' \
-	  '-lang=-:language:(english dansk deutsch espanol francais italiano nederlands norsk polski portugues suomi svenska turkce jiantizhongwen nihongo fantizhengwen hangul)'
+    _arguments $args \
+      '-search=-:search type:(web pics)' \
+      '-lang=-:language:(english dansk deutsch espanol francais italiano nederlands norsk polski portugues suomi svenska turkce jiantizhongwen nihongo fantizhengwen hangul)'
       ;;
       jake)
         _arguments $args \
@@ -386,20 +387,20 @@ case $state in
           :URL:_urls && ret=0
       ;;
       wayback)
-	_arguments $args \
-	  '-syear=-[start search from this year]:year' \
-	  '-smonth=-:[start search from this month]:month:(jan feb mar apr may jun jul aug sep oct nov dec)' \
-	  '-sday=-[start search from this day]:number' \
-	  '-eyear=-[end search in this year]:year' \
-	  '-emonth=-[end search in this month]:month:(jan feb mar apr may jun jul aug sep oct nov dec)' \
-	  '-eday=-[end search in this day]:number' \
-	  '-list[list all pages that match search criteria]' \
-	  '-dups[show dups]' \
-	  '-compare[compare pages]' \
-	  '-pdf[show as PDF]' \
-	  '-alias=-[how to handle site aliases]:alias:(merge show hide)' \
-	  '-redir=-[how to handle redirections]:redir:(hide flag show)' \
-	  '-type=-[file type to search for]:type:(image audio video binary text pdf)'
+    _arguments $args \
+      '-syear=-[start search from this year]:year' \
+      '-smonth=-:[start search from this month]:month:(jan feb mar apr may jun jul aug sep oct nov dec)' \
+      '-sday=-[start search from this day]:number' \
+      '-eyear=-[end search in this year]:year' \
+      '-emonth=-[end search in this month]:month:(jan feb mar apr may jun jul aug sep oct nov dec)' \
+      '-eday=-[end search in this day]:number' \
+      '-list[list all pages that match search criteria]' \
+      '-dups[show dups]' \
+      '-compare[compare pages]' \
+      '-pdf[show as PDF]' \
+      '-alias=-[how to handle site aliases]:alias:(merge show hide)' \
+      '-redir=-[how to handle redirections]:redir:(hide flag show)' \
+      '-type=-[file type to search for]:type:(image audio video binary text pdf)'
       ;;
       wetandwild)
         _arguments \
@@ -430,6 +431,6 @@ case $state in
     _wanted elvi expl elvi compadd \
       ${${${(f)"$(_call_program elvi surfraw -elvi)"}%%[[:space:]]##--*}%:*} && ret=0
   ;;
-esac  
+esac
 
 return ret
-- 
2.30.2


[-- Attachment #3: 0001-fix-ebay-completion-bug-due-to-tailing-space.patch~ --]
[-- Type: text/plain, Size: 1573 bytes --]

From dc4484bac85609faf5cc8057fc5d2b991f9b1c91 Mon Sep 17 00:00:00 2001
From: Marc Chantreux <eiro@phear.org>
Date: Sat, 10 Apr 2021 10:25:02 +0200
Subject: [PATCH] fix ebay completion bug due to tailing space

also remove the other tailing spaces
---
 Completion/Unix/Command/_surfraw | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Completion/Unix/Command/_surfraw b/Completion/Unix/Command/_surfraw
index f945f1ca9..b8a3033c0 100644
--- a/Completion/Unix/Command/_surfraw
+++ b/Completion/Unix/Command/_surfraw
@@ -165,7 +165,7 @@ case $state in
       ;;
       debsec)
         _message -e string 'package name, bug number or CVE ID'
-      ;; 
+      ;;
       deja)
         _arguments $args \
           '-results=-:[number of results to return]' \
@@ -183,7 +183,7 @@ case $state in
       ;;
       ebay)
         _arguments $args \
-          '-country=-:country:(com de uk fr)' \ 
+          '-country=-:country:(com de uk fr)' \
           '-results=-:[number of results to return]:number' && ret=0
       ;;
       etym)
@@ -233,7 +233,7 @@ case $state in
 	  '-title[search titles (default)]'
 	  '-author[search authors]'
 	  '-num[search etext numbers]'
-      ;;       
+      ;;
       imdb)
         _arguments $args \
           '-category=-:category:(All Titles MyMovies People Characters Quotes Bios Plots)' && ret=0
@@ -430,6 +430,6 @@ case $state in
     _wanted elvi expl elvi compadd \
       ${${${(f)"$(_call_program elvi surfraw -elvi)"}%%[[:space:]]##--*}%:*} && ret=0
   ;;
-esac  
+esac
 
 return ret
-- 
2.30.2

Re: 2 new patches for _surfraw

[Oliver Kiddle]

Marc Chantreux wrote:
> +++ b/Completion/Unix/Command/_surfraw
>  #compdef surfraw sr
> +# vim: et ts=2

We have a .editorconfig file in the root of the source tree that
declares this in an editor neutral manner. A plugin for vim exists to
use it. While I do use vim, I don't have, e.g. less configured for a
tabsize of 2. The expandtab option is fine but I go with shiftwidth=2
and leave the tabsize at the default 8.
Your patch even looks like you've fixed the tabs with the equivalent of
expand -t2

I have pushed the various patches including whitespace fixups. If
anything seems missing, let me know. Preferably pull from git for any
further patches to ensure they're against the latest state of HEAD.

Thanks

Oliver

Re: 2 new patches for _surfraw

[Marc Chantreux]

hello,

> I have pushed the various patches including whitespace fixups. If
> anything seems missing, let me know. Preferably pull from git for any
> further patches to ensure they're against the latest state of HEAD.

thanks a lot for your help. i'll take some time to learn about the completion
system  and implement our suggestions. thank you!

regards,
marc