Re: PATCH: _urls, _netscape, a seg fault and new completion thoughts

[Sven Wischnowsky <wischnow@xxxxxxxxxxxxxxxxxxxxxxx>]

Oliver Kiddle wrote:

> I had a bit of difficulty in getting the remote command completion to
> work properly with different types of quoting and there is probably
> plenty of scope for that to be further improved. I'm not sure how many
> of the problems with the quoting are my fault though: zsh will seg
> fault if I do:
> netscape -remote 'openURL(<tab>

Whew. When trying to reproduce this, I:

- first couldn't get the completion to do the right thing at all,
  because there was a buglet in the computil module
- after fixing this I found a bug in the parameter module which I
  fixed in utils.c: zwarnnam() was called with a NULL cmd argument,
  this is handled now by simply not trying to print a `cmd:'
- then I found a bug in sep_comp_sep(): the string in the quotes was
  tokenized which caused trouble in sep_comp_string()

Partly this was caused by `_netscape' not declaring enough local
parameters (`line' and `options' were missing). Or more precisely
because this caused the shell to set the `options' parameter from the
`parameter' module. Damn, I should have used another name, I knew this 
would cause trouble some time.

> Is there a good reason why I don't get a list of matches when
> completing inside quotes? This is the general case not anything to do
> with completing urls etc. e.g. cd 'm<tab> will complete to 'man/ but
> if I added another directory beginning with m, nothing would happen - no
> list.

Hm, this works for me -- unless you mean `cd man m<TAB>' which of
course shouldn't work.

> While writing this completion, I came across a number of circumstances
> where I wanted to pass certain compadd style options (e.g. -r, -s, -M)
> to _urls or _files but couldn't because they were not handled in some
> circumstances. As a way of making the new completions simpler, I think
> we should try to make all the completion commands standardise much more
> on their options so things such as -P -s -S -r can be used throughout
> with a common meaning and can be safely passed from one completion
> function to another.

`-P', `-S', and `-r': yes. `-s': no, this is needed by some functions
and I think calling functions shouldn't mess with them. For such
prefixes or suffixes they should use `I{PRE,SUF}FIX' (and `compset').

In fact, I tried to make the basic completion functions (the utility
functions) accept all options that may be interesting for calling
functions. Seems like I forgot to add `-r' when that was added.

> I think it would make things easier if compgen in particular took more
> of compadd's options. The problem is that most of the option letters
> are already used because compgen takes all the standard compctl
> options. Maybe we should strip compgen of many of the options
> which nolonger are of much use because they are implemented by new
> functions (e.g. -f -/ -g -u). I would even say we should consider
> ditching compgen altogether: it would fit in much better with the rest
> of the system where different functions complete different types of
> things if for example jobs were completed with compjob instead of the
> not very readable compgen -j, -z and -r. Alternatively, the option
> letters could be freed by moving compgen syntax away from the compctl
> style and use more full words to describe what it is completing -
> compgen suspended-jobs would be much easier to read.

I would be against a plethora of new builtins just to add completions
for only jobs and such. Also I don't like changing it to a full-word
argument style because no other buitlin does it this way. What I would 
like is to add more basic functions acting as wrappers around certain
compgen options (probably also giving interesting match specs for them 
and adding the `_description' stuff). We have already made steps
toward that, just look at functions like `_parameter', but not for
everything for which it makes sense (for some of compgen's options it
doesn't make sense because they are useful only for one or two
commands). The only problem is that noone seems to have found the time 
to do the work.
About disallowing some of the options: for some I did it from the
beginning. For other I'm not sure, mainly because I (and I guess by
now I'm really the only one) still think of the completion system as
an *example* (a rather biggish one). I'd like to not restrict the
possibilities of other completion system implementations by removing
options just because `we have already functions for it'.

*But*: I like to think of the moment where everyone uses the function
system and I can finally remove compctl altogether, i.e. that type of
interface, and clean up the code in tricky.c because there are
differences enough to cause trouble (and they already did because I
constantly forget to test the compctl style of completion).

So, I wouldn't be against removing `compgen' altogether... when we
finally have a way to produce all the matches that can be added by it
in another way. Actually I wrote the parameter module with that in
mind. But there are still things that aren't supported by it (jobs,
history, user names, and so on...). Once we have ways to produce
everything in a form we can give to compadd, I'll be happy to remove
compgen.

A complete list of the things compgen can do we can't easily generate
otherwise (preferably without the expensive `$(...)' constructs!)
might already help here, so if anyone finds the time...


Ok, this patch fixes the bugs described above. It also changes `_urls' 
and `_netscape' to use `_description' for the URL prefixes (folks,
please use it everywhere; for one, it looks weird if you suddenly get
a list without description after have gotten used to them, and then
a certain list member is currently thinking about combining it with
the tag stuff we have discussed -- which will make it more important).

Then I changed the names of the arrays used by `_arguments' and
`_values' to `opt_args' and `val_args' so that they at least don't
collide with the parameters in the parameter module.

Bye
 Sven

diff -u -r oldsrc/Zle/computil.c Src/Zle/computil.c
--- oldsrc/Zle/computil.c	Wed Oct 13 11:47:13 1999
+++ Src/Zle/computil.c	Wed Oct 13 14:41:11 1999
@@ -812,8 +812,8 @@
 		return p;
     } else {
 	for (p = d->opts; p; p = p->next)
-	    if (p->active && p->args && p->type != CAO_NEXT &&
-		strpfx(p->name, line)) {
+	    if (p->active && ((!p->args || p->type == CAO_NEXT) ?
+			      !strcmp(p->name, line) : strpfx(p->name, line))) {
 		if (end) {
 		    int l = strlen(p->name);
 
diff -u -r oldsrc/Zle/zle_tricky.c Src/Zle/zle_tricky.c
--- oldsrc/Zle/zle_tricky.c	Wed Oct 13 11:47:15 1999
+++ Src/Zle/zle_tricky.c	Wed Oct 13 15:23:01 1999
@@ -5452,6 +5452,7 @@
 
     if (compisuffix)
 	s = dyncat(s, compisuffix);
+    untokenize(s);
 
     return sep_comp_string("", s, lip + lp, 0);
 }
diff -u -r oldsrc/utils.c Src/utils.c
--- oldsrc/utils.c	Wed Oct 13 11:47:03 1999
+++ Src/utils.c	Wed Oct 13 14:53:58 1999
@@ -91,8 +91,10 @@
 	nicezputs(scriptname ? scriptname : argzero, stderr);
 	fputs(": ", stderr);
     }
-    nicezputs(cmd, stderr);
-    fputs(": ", stderr);
+    if (cmd) {
+	nicezputs(cmd, stderr);
+	fputs(": ", stderr);
+    }
     zerrmsg(fmt, str, num);
 }
 
diff -u olddoc/Zsh/compsys.yo Doc/Zsh/compsys.yo
--- olddoc/Zsh/compsys.yo	Wed Oct 13 14:25:15 1999
+++ Doc/Zsh/compsys.yo	Wed Oct 13 16:14:28 1999
@@ -863,7 +863,7 @@
 all leading and trailing spaces and tabs removed) will be stored in
 the global parameter tt(state) and the function returns with a return
 value of 300 (to make it distinguishable from other return values)
-after setting the global `tt(line)' and `tt(options)'
+after setting the global `tt(line)' and `tt(opt_args)'
 parameters as described below and without resetting any changes made
 to the special parameters such as tt(PREFIX) and tt(words).
 
@@ -894,7 +894,7 @@
 will be set to the command name and normal arguments from the command
 line, i.e. to the words from the command line excluding all options
 and their arguments. These are stored in the associative array
-`tt(options)', using the option names as keys and their arguments as
+`tt(opt_args)', using the option names as keys and their arguments as
 the values. For options that have more than one argument these are
 given as one string, separated by colons. All colons in the original
 arguments are preceded with backslashes.
@@ -1044,6 +1044,10 @@
 arguments) should be printed, the configuration key
 tt(describe_values) is used in the same way as the key
 tt(describe_options) is used by the tt(_arguments) function.
+
+One last difference is that this function uses the associative array
+tt(val_args) to report values and their arguments (but otherwise this
+is the same as the tt(opt_args) association used by tt(_arguments)).
 )
 item(tt(_regex_arguments))(
 This function is a compiler to generate a completion function.  The
diff -u -r oldcompletion/Base/_arguments Completion/Base/_arguments
--- oldcompletion/Base/_arguments	Wed Oct 13 11:47:42 1999
+++ Completion/Base/_arguments	Wed Oct 13 16:00:58 1999
@@ -171,18 +171,18 @@
       _description expl "$descr"
 
       if [[ "$action" = -\>* ]]; then
-        comparguments -W line options
+        comparguments -W line opt_args
         state="${${action[3,-1]##[ 	]#}%%[ 	]#}"
         compstate[restore]=''
         aret=yes
       else
         if [[ -z "$local" ]]; then
           local line
-          typeset -A options
+          typeset -A opt_args
           local=yes
         fi
 
-        comparguments -W line options
+        comparguments -W line opt_args
 
         if [[ "$action" = \ # ]]; then
 
diff -u -r oldcompletion/Base/_values Completion/Base/_values
--- oldcompletion/Base/_values	Wed Oct 13 11:47:43 1999
+++ Completion/Base/_values	Wed Oct 13 15:59:34 1999
@@ -59,14 +59,14 @@
       expl=( "-qS$sep" "$expl[@]" )
 
   if [[ "$action" = -\>* ]]; then
-    compvalues -v values
+    compvalues -v val_args
     state="${${action[3,-1]##[ 	]#}%%[ 	]#}"
     compstate[restore]=''
     return 1
   else
-    typeset -A values
+    typeset -A val_args
 
-    compvalues -v values
+    compvalues -v val_args
 
     if [[ "$action" = \ # ]]; then
 
diff -u -r oldcompletion/Linux/_rpm Completion/Linux/_rpm
--- oldcompletion/Linux/_rpm	Wed Oct 13 11:47:49 1999
+++ Completion/Linux/_rpm	Wed Oct 13 16:03:09 1999
@@ -44,7 +44,7 @@
 # Used by `_arguments', made local here.
 
 local state lstate line
-typeset -A options
+typeset -A opt_args
 
 state=''
 
diff -u -r oldcompletion/User/_flex Completion/User/_flex
--- oldcompletion/User/_flex	Wed Oct 13 14:25:27 1999
+++ Completion/User/_flex	Wed Oct 13 16:03:37 1999
@@ -1,7 +1,7 @@
 #compdef flex
 
 local state line ret=1
-typeset -A options
+typeset -A opt_args
 
 _arguments -s \
   --help --version \
diff -u -r oldcompletion/User/_gcc Completion/User/_gcc
--- oldcompletion/User/_gcc	Wed Oct 13 14:25:27 1999
+++ Completion/User/_gcc	Wed Oct 13 16:03:47 1999
@@ -1,7 +1,7 @@
 #compdef gcc
 
 local state line ret=1 expl args
-typeset -A options
+typeset -A opt_args
 
 args=()
 case $MACHTYPE in
diff -u -r oldcompletion/User/_gprof Completion/User/_gprof
--- oldcompletion/User/_gprof	Wed Oct 13 14:25:28 1999
+++ Completion/User/_gprof	Wed Oct 13 16:04:03 1999
@@ -1,7 +1,7 @@
 #compdef gprof
 
 local state line ret=1
-typeset -A options
+typeset -A opt_args
 
 _arguments -s -{a,b,c,D,h,i,l,L,s,T,v,w,x,y,z} \
            -{A,C,e,E,f,F,J,n,N,O,p,P,q,Q,Z}:'function name:->funcs' \
diff -u -r oldcompletion/User/_gs Completion/User/_gs
--- oldcompletion/User/_gs	Wed Oct 13 14:25:28 1999
+++ Completion/User/_gs	Wed Oct 13 16:04:16 1999
@@ -9,7 +9,7 @@
   fi
 else
   local state line ret=1
-  typeset -A options
+  typeset -A opt_args
 
   _x_arguments \
     '-q[quiet startup]' \
diff -u -r oldcompletion/User/_lynx Completion/User/_lynx
--- oldcompletion/User/_lynx	Wed Oct 13 14:25:29 1999
+++ Completion/User/_lynx	Wed Oct 13 16:04:31 1999
@@ -1,7 +1,7 @@
 #compdef lynx
 
 local state line
-typeset -A options
+typeset -A opt_args
 
 _arguments \
   '-accept_all_cookies' \
diff -u -r oldcompletion/User/_mount Completion/User/_mount
--- oldcompletion/User/_mount	Wed Oct 13 14:25:29 1999
+++ Completion/User/_mount	Wed Oct 13 16:05:46 1999
@@ -6,7 +6,7 @@
 # are below these table.
 
 local state line ret=1 args fss deffs=iso9660 descr tmp
-typeset -A options
+typeset -A opt_args
 
 if (( ! $+_fs_any )); then
 
@@ -233,30 +233,30 @@
   compadd "$expl[@]" -qS, -M 'L:|no=' - "$fss[@]" && ret=0
   ;;
 fsopt)
-  eval 'tmp=(' '"$_fs_'${(s:,:)^${options[-t]:-${deffs}}}'[@]"' ')'
+  eval 'tmp=(' '"$_fs_'${(s:,:)^${opt_args[-t]:-${deffs}}}'[@]"' ')'
   tmp=( "$_fs_any[@]" "${(@)tmp:#}" )
   _values -s , 'file system options' "$tmp[@]" && ret=0
   ;;
 devordir)
-  if (( $+options[-a] )); then
+  if (( $+opt_args[-a] )); then
     _message "no device or directory with option \`-a'"
   else
     _description expl device
     compadd "$expl[@]" /dev/* && ret=0
-    if (( ! $+options[-t] )); then
+    if (( ! $+opt_args[-t] )); then
       _description expl 'mount point'
       _files "$expl[@]" -/ && ret=0
     fi
   fi
   ;;
 udevordir)
-  if (( $+options[-a] )); then
+  if (( $+opt_args[-a] )); then
     _message "no device or directory with option \`-a'"
   else
     tmp=( "${(@f)$(< /etc/mtab)}" )
     _description expl device
     compadd "$expl[@]" - "${(@)${(@)tmp%% *}:#none}" && ret=0
-    if (( ! $+options[-t] )); then
+    if (( ! $+opt_args[-t] )); then
       _description expl 'mount point'
       compadd "$expl[@]" - "${(@)${(@)tmp#* }%% *}"
     fi
diff -u -r oldcompletion/User/_netscape Completion/User/_netscape
--- oldcompletion/User/_netscape	Wed Oct 13 14:25:29 1999
+++ Completion/User/_netscape	Wed Oct 13 16:14:08 1999
@@ -1,6 +1,7 @@
 #compdef netscape
 
-local state
+local state line
+typeset -A opt_args
 
 _x_arguments \
   '-xrm:resource:_x_resource' \
@@ -69,7 +70,8 @@
     compadd authors blank cache document fonts global hype image-cache \
         license logo memory-cache mozilla plugins
   else
-    compadd -S '' about: mocha: javascript:
+    _description expl 'URL prefix'
+    compadd "$expl[@]" -S '' about: mocha: javascript:
     _urls "$@"
   fi
 fi
diff -u -r oldcompletion/User/_nslookup Completion/User/_nslookup
--- oldcompletion/User/_nslookup	Wed Oct 13 14:25:29 1999
+++ Completion/User/_nslookup	Wed Oct 13 16:06:15 1999
@@ -113,7 +113,7 @@
     return
     ;;
   set)
-    typeset -A values
+    typeset -A val_args
 
     _values 'state information' "$setopts[@]" && ret=0
 
@@ -130,7 +130,7 @@
 
 if [[ -z "$state" ]]; then
   local line
-  typeset -A options
+  typeset -A opt_args
 
   _arguments \
     "-${(@)^${(@M)setopts:#*\]:*}/\[/=[}" \
diff -u -r oldcompletion/User/_rlogin Completion/User/_rlogin
--- oldcompletion/User/_rlogin	Wed Oct 13 14:25:30 1999
+++ Completion/User/_rlogin	Wed Oct 13 16:07:07 1999
@@ -21,7 +21,7 @@
     ;;
   rsh|remsh)
     local state line ret=1
-    typeset -A options
+    typeset -A opt_args
 
     _arguments -s \
       '-n[ignore stdin]' \
@@ -39,7 +39,7 @@
     ;;
   rcp)
     local state line ret=1
-    typeset -A options
+    typeset -A opt_args
 
     _arguments -s \
       '-p[preserve modification times]' \
@@ -71,7 +71,7 @@
     _combination accounts_users_hosts "users=${IPREFIX/@}" hosts "$@"
   else
     _combination accounts_users_hosts \
-      ${options[-l]:+"users=${options[-l]:q}"} hosts "$@"
+      ${opt_args[-l]:+"users=${opt_args[-l]:q}"} hosts "$@"
   fi
 }
 
diff -u -r oldcompletion/User/_socket Completion/User/_socket
--- oldcompletion/User/_socket	Wed Oct 13 14:25:30 1999
+++ Completion/User/_socket	Wed Oct 13 16:07:36 1999
@@ -6,7 +6,7 @@
 #    The array that contains paris `host:port'.
 
 local state line expl
-typeset -A options
+typeset -A opt_args
 
 if [[ $CURRENT -eq 2 && (
   -z "$compconfig[option_prefix]" ||
@@ -41,7 +41,7 @@
   ;;
 
 arg1)
-  if (( $+options[-s] )); then
+  if (( $+opt_args[-s] )); then
     _description expl 'port to listen'
     _ports "$expl[@]"
   else
@@ -51,7 +51,7 @@
   ;;
 
 arg2)
-  if (( ! $+options[-s] )); then
+  if (( ! $+opt_args[-s] )); then
     _description expl 'port to connect'
     _combination socket_hosts_ports hosts="${line[2]:q}" ports "$expl[@]"
   fi
diff -u -r oldcompletion/User/_ssh Completion/User/_ssh
--- oldcompletion/User/_ssh	Wed Oct 13 14:25:31 1999
+++ Completion/User/_ssh	Wed Oct 13 16:08:03 1999
@@ -2,7 +2,7 @@
 
 _ssh () {
   local state lstate line ret=1 expl args
-  typeset -A options
+  typeset -A opt_args
 
   local accounts_users_hosts
 
@@ -143,7 +143,7 @@
         else
           _description expl 'remote host name'
           _ssh_hosts "$expl[@]" && ret=0
-          if (( ! $+options[-l] )); then
+          if (( ! $+opt_args[-l] )); then
             _description expl 'login name'
             _ssh_users "$expl[@]" -S@ -q && ret=0
           fi
@@ -230,7 +230,7 @@
     _combination accounts_users_hosts "users=${IPREFIX/@}" hosts "$@"
   else
     _combination accounts_users_hosts \
-      ${options[-l]:+"users=${options[-l]:q}"} hosts "$@"
+      ${opt_args[-l]:+"users=${opt_args[-l]:q}"} hosts "$@"
   fi
 }
 
diff -u -r oldcompletion/User/_telnet Completion/User/_telnet
--- oldcompletion/User/_telnet	Wed Oct 13 14:25:31 1999
+++ Completion/User/_telnet	Wed Oct 13 16:08:29 1999
@@ -6,7 +6,7 @@
 #    The array that contains 3-tuples `host:port:user'.
 
 local state line expl
-typeset -A options
+typeset -A opt_args
 
 if (( ! $+_telnet_short )); then
   local k help="$(telnet -\? < /dev/null 2>&1)"
@@ -69,14 +69,14 @@
 hosts)
   _description expl 'host'
   _combination telnet_hosts_ports_users \
-    ${options[-l]:+users=${options[-l]:q}} \
+    ${opt_args[-l]:+users=${opt_args[-l]:q}} \
     hosts "$expl[@]"
   ;;
 
 ports)
   _description expl 'port'
   _combination telnet_hosts_ports_users \
-    ${options[-l]:+users=${options[-l]:q}} \
+    ${opt_args[-l]:+users=${opt_args[-l]:q}} \
     hosts="${line[2]:q}" \
     ports "$expl[@]"
   ;;
diff -u -r oldcompletion/User/_urls Completion/User/_urls
--- oldcompletion/User/_urls	Wed Oct 13 14:25:31 1999
+++ Completion/User/_urls	Wed Oct 13 15:29:52 1999
@@ -37,7 +37,7 @@
 #    name used by a user placing web pages within their home area.
 #    e.g. compconf urls_localhttp=www:/usr/local/apache/htdocs:public_html
 
-local ipre scheme host user dirs files ret=1
+local ipre scheme host user dirs files ret=1 expl
 
 if [[ "$1" = -f ]]; then
   shift
@@ -54,9 +54,10 @@
   scheme="${PREFIX%%:*}"
   compset -P "[-+.a-z0-9]#:"
 else
-  [ -d $compconfig[urls_path]/bookmark ] &&
-      compadd "$@" -S '' bookmark: && ret=0
-  compadd "$@" -S '' file: ftp:// gopher:// http:// && ret=0
+  _description expl 'URL prefix'
+  [[ -d $compconfig[urls_path]/bookmark ]] &&
+      compadd "$@" "$expl[@]" -S '' bookmark: && ret=0
+  compadd "$@" "$expl[@]" -S '' file: ftp:// gopher:// http:// && ret=0
   return $ret
 fi
 
diff -u -r oldcompletion/User/_wget Completion/User/_wget
--- oldcompletion/User/_wget	Wed Oct 13 14:25:32 1999
+++ Completion/User/_wget	Wed Oct 13 16:09:03 1999
@@ -1,7 +1,7 @@
 #compdef wget
 
 local state line
-typeset -A options
+typeset -A opt_args
 
 local tmp1 tmp2
 
diff -u -r oldcompletion/User/_yp Completion/User/_yp
--- oldcompletion/User/_yp	Wed Oct 13 14:25:32 1999
+++ Completion/User/_yp	Wed Oct 13 16:09:18 1999
@@ -1,7 +1,7 @@
 #compdef ypcat ypmatch yppasswd ypwhich ypset ypserv ypbind yppush yppoll ypxfr domainname
 
 local line state ret=1
-typeset -A options
+typeset -A opt_args
 
 if (( ! $+_yp_cache_maps )); then
   _yp_cache_maps=( "${(@)${(@f)$(ypwhich -m)}%% *}" )
@@ -93,7 +93,7 @@
 
   compadd "$expl[@]" -M 'l:.|by=by l:.|=by r:|.=* r:|=*' - \
           "$_yp_cache_maps[@]" && ret=0
-  if [[ $+options[-t] -eq 0 && "$state" != maponly ]]; then
+  if [[ $+opt_args[-t] -eq 0 && "$state" != maponly ]]; then
     _description expl 'nicknames'
     compadd "$expl[@]" - "$_yp_cache_nicks[@]" && ret=0
   fi
diff -u -r oldcompletion/X/_xmodmap Completion/X/_xmodmap
--- oldcompletion/X/_xmodmap	Wed Oct 13 11:47:56 1999
+++ Completion/X/_xmodmap	Wed Oct 13 16:09:32 1999
@@ -3,7 +3,7 @@
 setopt localoptions extendedglob
 
 local state line ret=1
-typeset -A options
+typeset -A opt_args
 
 _x_arguments \
   -{help,grammar,verbose,quiet} \


--
Sven Wischnowsky                         wischnow@xxxxxxxxxxxxxxxxxxxxxxx