_call_program (and possibly other hooks) or opt_args quoting prob lem.

_call_program (and possibly other hooks) or opt_args quoting prob lem.

[Borsenkow Andrej]

_call_program evals its argument(s). It creates very interesting problem -
we want to quote word separator _but_ we do not want to quote possible
parameter expansions ... to illustrate:

trying to complete

info -d $PWD/a\ b TAB

you either do not quote value of option -d, getting

+_call_program:12> eval info -d $PWD/a b --output -

wrong because it splits filename

or you do quote value of option -d getting

+_call_program:12> eval info -d \$PWD/a\ b --output -

which is wrong as well because it does not expand $PWD.

The problem seems to actually be in how _arguments stores values of options.
It seems to remove all quotes from them, but then, we cannot quote them back
reliably. It really looks like _argument should save word(s) from command
line verbatim and left to user to decide when values need to be dequoted.

OTOH it means that user possibly must pass values via eval every time ...
horrors. Just assume we need real directory name ... [ -d ${opt_args[-d]} ]
would be totally wrong.

-andrej

Re: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Bart Schaefer]

On May 16,  8:32pm, Borsenkow Andrej wrote:
}
} _call_program evals its argument(s). It creates very interesting problem -
} we want to quote word separator _but_ we do not want to quote possible
} parameter expansions ...

What about this?

Index: Completion/Base/Utility/_call_program
===================================================================
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 _call_program
--- Completion/Base/Utility/_call_program	2001/04/09 20:14:08	1.1.1.1
+++ Completion/Base/Utility/_call_program	2002/05/17 18:38:04
@@ -4,7 +4,7 @@
 
 if zstyle -s ":completion:${curcontext}:${1}" command tmp; then
   if [[ "$tmp" = -* ]]; then
-    eval "$tmp[2,-1]" "$argv[2,-1]"
+    eval "$tmp[2,-1]" "${(qqq)argv[2,-1]}"
   else
     eval "$tmp"
   fi



-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

RE: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Borsenkow Andrej]

> 
> On May 16,  8:32pm, Borsenkow Andrej wrote:
> }
> } _call_program evals its argument(s). It creates very interesting
problem -
> } we want to quote word separator _but_ we do not want to quote
possible
> } parameter expansions ...
> 
> What about this?
> 
> Index: Completion/Base/Utility/_call_program
> ===================================================================
> retrieving revision 1.1.1.1
> diff -c -r1.1.1.1 _call_program
> --- Completion/Base/Utility/_call_program	2001/04/09 20:14:08
1.1.1.1
> +++ Completion/Base/Utility/_call_program	2002/05/17 18:38:04
> @@ -4,7 +4,7 @@
> 
>  if zstyle -s ":completion:${curcontext}:${1}" command tmp; then
>    if [[ "$tmp" = -* ]]; then
> -    eval "$tmp[2,-1]" "$argv[2,-1]"
> +    eval "$tmp[2,-1]" "${(qqq)argv[2,-1]}"
>    else
>      eval "$tmp"
>    fi
> 

It won't work in this form; it needs at least ${(@qqq)... and in other
branch. Still even with the following:

if zstyle -s ":completion:${curcontext}:${1}" command tmp; then
  if [[ "$tmp" = -* ]]; then
    eval "$tmp[2,-1]" "${(qqq)argv[2,-1]}"
  else
    eval "$tmp"
  fi
else
  eval "${(@qqq)argv[2,-1]}"
fi


I get 

+_info:27> info=( _call_program info info -d $PWD/a b ) 
+_info:29> items=+_info:1> _call_program info info -d $PWD/a b --output
-
+_call_program:3> local tmp
+_call_program:5> zstyle -s :completion::complete:info::info command tmp
+_call_program:12> eval "info" "-d" "\$PWD/a b" "--output" "-"

Remember, (qqq) _quotes_ arguments (using different rules but it does
not matter).

Actually I begin to feel that _arguments should eval parameter value
before storing it. Can anybody give example when parameter value as it
appears on command line is useful? 

-andrej

Re: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Bart Schaefer]

On May 18,  3:24pm, Borsenkow Andrej wrote:
}
} Still even with the following:
} 
} if zstyle -s ":completion:${curcontext}:${1}" command tmp; then
}   if [[ "$tmp" = -* ]]; then
}     eval "$tmp[2,-1]" "${(qqq)argv[2,-1]}"
}   else
}     eval "$tmp"
}   fi
} else
}   eval "${(@qqq)argv[2,-1]}"
} fi
} 
} 
} I get 
} 
} +_info:27> info=( _call_program info info -d $PWD/a b ) 

I don't have this _info completion function, so I'm having a hard time
coming up with a way to reproduce this.

Perhaps "${(@qqq)${(@e)argv[2,-1]}}" ?

Or perhaps drop the `eval' and just use "${(@e)argv[2,-1]}}" directly in
the latter branch -- the only reason I can think of for the `eval' to be
there is to expand aliases, and I wouldn't think we'd want to do that when
inside _call_program, but I could be wrong.

} Actually I begin to feel that _arguments should eval parameter value
} before storing it. Can anybody give example when parameter value as it
} appears on command line is useful? 

I'm not sure what you mean by "parameter value as it appears" -- is the
word "value" out of place in that phrase?

What about the _expand completer?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Borsenkow Andrej]

В Пнд, 20.05.2002, в 20:40, Bart Schaefer написал:
> 
> I don't have this _info completion function, so I'm having a hard time
> coming up with a way to reproduce this.
> 

You do no need to. Just do:

_foo() {
    local context state line ret=1
    typeset -A opt_args

    _arguments \
        '-d[directory]:directory:_files -/' \
        '1:test_arg:->test' && ret=0

    case "$state" in
        test )
            local -a opts
            (( $+opt_args[-d] )) && opts=(-W $opt_args[-d])
            _files $opts && ret=0
        ;;
    esac

    return $ret
}

mkdir a\ b
touch a\ b/test

and try to complete

foo -d $PWD/a\ b TAB


> Perhaps "${(@qqq)${(@e)argv[2,-1]}}" ?
> 
> Or perhaps drop the `eval' and just use "${(@e)argv[2,-1]}}" directly in
> the latter branch -- the only reason I can think of for the `eval' to be
> there is to expand aliases, and I wouldn't think we'd want to do that when
> inside _call_program, but I could be wrong.
> 
> } Actually I begin to feel that _arguments should eval parameter value
> } before storing it. Can anybody give example when parameter value as it
> } appears on command line is useful? 
> 
> I'm not sure what you mean by "parameter value as it appears" -- is the
> word "value" out of place in that phrase?
>

Once more:

user enters

$PWD/a\ b

as the "value" of option -d. When this is finally passed to a program
for execution, shell expands $PWD so program actually sees (remove
quotes) "/home/bor/test/a b". This is "real" value of option -d.
Alternatively, we can store it as it appears on the line - i.e. "$PWD/a\
b". _Then_ _call_program would work just fine - by eval'ing this you'd
get exactly the correct value.


What happens currently is, zsh removes quoting when it stores values in
opt_args array but it does not expand it. After this is done there is no
way to recover original string as entered by user. So in cases when you
may want it (see above example _foo) you have no means to get real
value.

Look in trace for above example:

+_foo:11> opts=( -W $PWD/a b )

how do we know that space has been quoted but dollar in $PWD was not?
Any quoting that you apply now will be equally applied to both space and
$ - while you need to leave $ unquoted (for eval).

 
> What about the _expand completer?
> 

The problem is not to complete the value of option (this works just
fine). The problem is to use this value in completing other parts of
command.

I am sorry if it is confusing but I really do not understand what you do
not understand ... it seems pretty clear to me :-)

-andrej

Re: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Sven Wischnowsky]

Borsenkow Andrej wrote:

> ...
> 
> What happens currently is, zsh removes quoting when it stores values in
> opt_args array but it does not expand it. After this is done there is no
> way to recover original string as entered by user. So in cases when you
> may want it (see above example _foo) you have no means to get real
> value.

It's the code affected by the patch below (which I'm not going to
commit right now, only if I get replies). We need the de-quoting
because options may be partly quoted (Bart once send a message about
this and as a consequence I added the de-quoting code).

I'm not sure if we should commit this patch, because, as was already
pointed out, ${(e)...} on a string as it is reported now should give
one the needed expansion. With the patch that would become ${(Qe)...}.

Any opinions?


Bye
  Sven

P.S.: The patch may be offset, never mind.

diff -ur -r ../oz/Src/Zle/computil.c ./Src/Zle/computil.c
--- ../oz/Src/Zle/computil.c	Sat May 18 20:31:04 2002
+++ ./Src/Zle/computil.c	Sat May 18 23:28:25 2002
@@ -1693,6 +1695,33 @@
     zfree(s->oargs, s->d->nopts * sizeof(LinkList));
 }
 
+/* Return a copy of an option's argument, ignoring possible quoting
+ * in the option name. */
+
+static char *
+ca_opt_arg(Caopt opt, char *line)
+{
+    char *o = opt->name;
+
+    while (1) {
+        if (*o == '\\')
+            o++;
+        if (*line == '\\' || *line == '\'' || *line == '"')
+            line++;
+        if (!*o || *o != *line)
+            break;
+        o++;
+        line++;
+    }
+    if (*line && (opt->type == CAO_EQUAL || opt->type == CAO_OEQUAL)) {
+        if (*line == '\\')
+            line++;
+        if (*line == '=')
+            line++;
+    }
+    return ztrdup(line);
+}
+
 /* Parse a command line. */
 
 static int
@@ -1701,7 +1730,7 @@
     Caarg adef, ddef;
     Caopt ptr, wasopt = NULL, dopt;
     struct castate state;
-    char *line, *pe, **argxor = NULL;
+    char *line, *oline, *pe, **argxor = NULL;
     int cur, doff, argend, arglast, ne;
     Patprog endpat = NULL, napat = NULL;
     LinkList sopts = NULL;
@@ -1767,6 +1796,7 @@
 	doff = state.singles = arglast = 0;
 
         /* remove quotes */
+        oline = line;
         line = dupstring(line);
         ne = noerrs;
         noerrs = 2;
@@ -1786,7 +1816,7 @@
 	if (state.def) {
 	    state.arg = 0;
 	    if (state.curopt)
-		zaddlinknode(state.oargs[state.curopt->num], ztrdup(line));
+		zaddlinknode(state.oargs[state.curopt->num], ztrdup(oline));
 
 	    if ((state.opt = (state.def->type == CAA_OPT)) && state.def->opt)
 		state.oopt++;
@@ -1868,7 +1898,8 @@
 		    state.def->type != CAA_RREST)
 		    state.def = state.def->next;
 
-		zaddlinknode(state.oargs[state.curopt->num], ztrdup(pe));
+		zaddlinknode(state.oargs[state.curopt->num],
+                             ca_opt_arg(state.curopt, oline));
 	    }
 	    if (state.def)
 		state.opt = 0;
@@ -1921,7 +1952,8 @@
 		    state.def->type != CAA_RREST)
 		    state.def = state.def->next;
 
-		zaddlinknode(state.oargs[state.curopt->num], ztrdup(pe));
+		zaddlinknode(state.oargs[state.curopt->num],
+                             ca_opt_arg(state.curopt, line));
 	    }
 	    if (state.def)
 		state.opt = 0;

-- 
Sven Wischnowsky                          wischnow@berkom.de

Re: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Bart Schaefer]

On May 20,  9:43pm, Borsenkow Andrej wrote:
} Subject: Re: _call_program (and possibly other hooks) or opt_args quoting 
}
} > I'm not sure what you mean by "parameter value as it appears" -- is the
} > word "value" out of place in that phrase?
} 
} Once more:
} 
} user enters
} 
} $PWD/a\ b
} 
} as the "value" of option -d. When this is finally passed to a program
} for execution, shell expands $PWD so program actually sees (remove
} quotes) "/home/bor/test/a b". This is "real" value of option -d.
[...]
} I am sorry if it is confusing but I really do not understand what you do
} not understand ... it seems pretty clear to me :-)

I get it now.  The word "parameter" was what confused me -- zsh's use of
"parameter" to refer to shell variables like $PWD has always been a pain
in the brain.  Anyway, I couldn't tell whether you were talking about the
value of the parameter $PWD, or the presence of a parameter expansion in
the command-line arguments, or what.

On May 21, 10:14am, Sven Wischnowsky wrote:
} 
} I'm not sure if we should commit this patch, because, as was already
} pointed out, ${(e)...} on a string as it is reported now should give
} one the needed expansion. With the patch that would become ${(Qe)...}.
} 
} Any opinions?

Having looked at Andrej's example with _files, I think you should commit
the patch.  It would appear that the number of cases where we'd need to
insert uses of ${(e)...} without the patch, exceeds the number of cases
where we'd need to insert ${(Qe)...} with it.

However, that opinion is based on very superficial analysis, so if anyone
thinks otherwise ...

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: _call_program (and possibly other hooks) or opt_args quoting prob lem.

[Sven Wischnowsky]

Bart Schaefer wrote:

> ...
> 
> On May 21, 10:14am, Sven Wischnowsky wrote:
> } 
> } I'm not sure if we should commit this patch, because, as was already
> } pointed out, ${(e)...} on a string as it is reported now should give
> } one the needed expansion. With the patch that would become ${(Qe)...}.
> } 
> } Any opinions?
> 
> Having looked at Andrej's example with _files, I think you should commit
> the patch.  It would appear that the number of cases where we'd need to
> insert uses of ${(e)...} without the patch, exceeds the number of cases
> where we'd need to insert ${(Qe)...} with it.
> 
> However, that opinion is based on very superficial analysis, so if anyone
> thinks otherwise ...

I had the same feeling anyway, I'm committing it now.


Bye
  Sven

-- 
Sven Wischnowsky                          wischnow@berkom.de