Re: destructive list-expand

[Sven Wischnowsky <wischnow@xxxxxxxxxxxxxxxxxxxxxxx>]

Bart Schaefer wrote:

> On May 15,  4:18pm, Sven Wischnowsky wrote:
> } Subject: Re: destructive list-expand
> }
> } Peter Stephenson wrote:
> } 
> } > Sven wrote:
> } > > The patch makes it keep all quotes *inside* parameter expansions when
> } > > *not* completing a parameter name.  For the simple things I tried, this
> } > > worked.  And for the case we were discussing, this gives the right
> } > > string to the shell code.
> 
> This now gives the right thing for me with list-expand, but still fails
> on _list_expansions (or _expand_word).  The command line isn't de-quoted
> in either case, but _expand_word still just feeps at me.
> 
> schaefer<506> echo ${(M)${(f)"$(<=(print -l *))"}:#*conf*}<TAB>
> No matches for `file' or `corrections'
> schaefer<506> echo ${(M)${(f)"$(<=(print -l *))"}:#*conf*}<C-x g>
> acconfig.h    config.h.in   config.sub    configure.in  
> config.guess  config.log    configure     

Yes, I pointed that out and the reason for it: _expand uses

      eval 'exp=( ${${(e)exp//\\[ 	
    ]/ }//(#b)([ 	
    ])/\\$match[1]} )' 2>/dev/null

But that doesn't work here:

    beta% echo ${(M)${(f)"$(<=(print -l *))"}:#*conf*}
    acconfig.h conf config.cache config.guess config.h config.h.in config.log config.moduls config.status config.sub configure configure.in sconf
    beta% foo='${(M)${(f)"$(<=(print -l *))"}:#*conf*}'
    beta% echo ${(e)foo}
 
    beta% echo ${(e)~foo}
    CVS ChangeLog ChangeLog-Release ChangeLog.3.0 ChangeLog~ Completion Config Doc Etc Functions INSTALL LICENCE META-FAQ Makefile Makefile.in Misc README Src StartupFiles Test Util a acconfig.h aclocal.m4 aczsh.m4 conf config.cache config.guess config.h config.h.in config.log config.modules config.status config.sub configure configure.in core install-sh mkinstalldirs sconf so_locations stamp-h stamp-h.in

I don't know if this is a bug (it looks like one, in a certain sense),
nor do I know how to fix this (at least without looking at the parameter
code again).

> Then there's this problem -- move the quotes outside the parameter:
> 
> schaefer<507> echo "${(M)${(f)$(<=(print -l *))}:#*conf*}"<C-x g>
> [expansion elided]
> schaefer<507> echo ${(M)${(f)$(<=(print -l *))}:#*conf*}"
> 
> Now the leading quote has been removed, but the trailing quote is there.
> (They used to both disappear.)

The real reason for this was that my change didn't look out for
Qstring's, only for String tokens, fix below.

> } > Are we sure the zle_tricky.c change doesn't cause some knock-on problem,
> } > e.g. in old-style completion? 
> 
> Obviously we're not.

Yes, but it was already broken before. Still, I offer to back out the
last patch and this one if you or Peter decide that you prefer it.


In another mail:

> On May 15,  2:41pm, Bart Schaefer wrote:
> } Subject: Re: destructive list-expand
> }
> } schaefer<507> echo "${(M)${(f)$(<=(print -l *))}:#*conf*}"<C-x g>
> } [expansion elided]
> } schaefer<507> echo ${(M)${(f)$(<=(print -l *))}:#*conf*}"
> } 
> } Now the leading quote has been removed, but the trailing quote is there.
> } (They used to both disappear.)
> 
> Just to be clear ... both of them disappearing is wrong too, of course.

Yes.

> Why is it ever correct to delete quotes _anywhere_?

Think about *completion*, not expansion. There we *need* to remove the
quotes at some time to be able to complete `"foo<TAB>' and things like
that. We had quite a bit of discussion about all this quoting stuff
which I don't want to revive. The result was that we want to try to keep
starting single and double quotes if possible and otherwise use what
could probably be called a normalised quoted form (backslashes instead
of single or double quotes). I had several very miserable days to at
least reach the state we have now.

> Here's an even worse error:
> 
> schaefer<510> echo '${(@M)${(f)$(<=(print -l *))}:#*conf*}'<C-x g>
> schaefer<510> echo ${(@M)${(f)$(<=(print -l *))}:#*conf*}
> 
> Now both *single* quotes are gone.  Bad news.

The patch below just blindly changes the command line back to the
original when expansion didn't change anything (because we were only
listing or because the word couldn't be expanded).


Anyway, here's the patch which hopefully fixes at least most problems.
The expansion stuff in _expand is not yet handled.


And all this is really ugly to work on. That's why I said I would like
to move more stuff into shell code, which might make this much easier to
fix.


Bye
  Sven

Index: Src/Zle/zle_tricky.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_tricky.c,v
retrieving revision 1.26
diff -u -r1.26 zle_tricky.c
--- Src/Zle/zle_tricky.c	2001/05/15 13:52:23	1.26
+++ Src/Zle/zle_tricky.c	2001/05/16 10:22:22
@@ -783,8 +783,20 @@
 			}
 		}
 		ret = docompletion(s, lst, lincmd);
-	    } else if (ret)
-		clearlist = 1;
+            } else {
+                if (ret)
+                    clearlist = 1;
+                if (!strcmp(ol, (char *)line)) {
+                    /* We may have removed some quotes. For completion, other
+                     * parts of the code re-install them, but for expansion
+                     * we have to do it here. */
+                    cs = 0;
+                    foredel(ll);
+                    spaceinline(origll);
+                    memcpy(line, origline, origll);
+                    cs = origcs;
+                }
+            }
 	} else
 	    /* Just do completion. */
 	    ret = docompletion(s, lst, lincmd);
@@ -1348,7 +1360,7 @@
                 *p = '\'';
             else if (level && *p == Dnull)
                 *p = '"';
-            else if (*p == String && p[1] == Inbrace)
+            else if ((*p == String || *p == Qstring) && p[1] == Inbrace)
                 level++;
             else if (*p == Outbrace)
                 level--;
@@ -1722,9 +1734,15 @@
 	goto end;
     }
     if (lst == COMP_LIST_EXPAND) {
-	/* Only the list of expansions was requested. */
-	ret = listlist(vl);
-	showinglist = 0;
+	/* Only the list of expansions was requested. Restore the 
+         * command line. */
+        cs = 0;
+        foredel(ll);
+        spaceinline(origll);
+        memcpy(line, origline, origll);
+        cs = origcs;
+        ret = listlist(vl);
+        showinglist = 0;
 	goto end;
     }
     /* Remove the current word and put the expansions there. */

-- 
Sven Wischnowsky                         wischnow@xxxxxxxxxxxxxxxxxxxxxxx