Remaining zsh3.0-pre2 bugs

Remaining zsh3.0-pre2 bugs

[Huy Le]

Here are some bugs that I've reported but still remain in the latest
version of zsh.
The apply for both HP-UX 9.01 and IRIX 5.3

------------------------------------------------------------------------------
1. History bug:
The magic-space binding seems to mess up the history of
a line with a quoted argument that spawns two lines.
Note that in the following examples the number of spaces
and the placement of the single-quote matter.

pride% exec ./zsh -f
pride% echo 'a
> b c'
a
b c
pride% history
    1  echo 'a\nb c'
    2  history
pride% echo 'a
> b' c
a
b c
pride% history
    1  echo 'a\nb c'
    2  history
    3  echo 'a\nb' c
    4  history
pride% exec ./zsh -f
pride% bindkey ' ' magic-space
pride% echo 'a
> b c'
a
b c
pride% history
    1  bindkey ' ' magic-space
    2  history
pride% echo 'a
> b' c
a
b c
pride% history
    1  bindkey ' ' magic-space
    2  history
    3   c
    4  history

------------------------------------------------------------------------------
2. tty bug
When running either of the two sequences:
  while echo crap >/dev/null; do done
  while true; do echo crap >/dev/null; done
and interrupting them with <CTRL>-C, my output gets lost from then on.
This is an intermittent problem on HP-UX--keep doing it and eventually
it'll happen. On IRIX, the bug has happened to me every time.

pride% while echo crap >/dev/null; do done
pride% echo yes
pride% exec >/dev/tty
pride% echo yes
yes
pride%

------------------------------------------------------------------------------
3. tty bug #2
When tty output of "less" or "more" is suspended because it's in the
background, after "fg"ing my tty settings are reset to default.
This problem is hard because on HP-UX it is intermittent; happens more
regularly on IRIX.  Also, enough output must be generated (hence ls /bin)
so that you can quit out of "less" in the middle of the output.
As a semi-test that this is a zsh bug, I ran the same sequences of
commands under tcsh, and didn't get the bug.

acro% exec ./zsh -f
acro% stty eof '^D'
acro% stty -a
speed 38400 baud; line = 1;
intr = ^C; quit = ^@; erase = DEL; kill = ^U; eof = ^D; eol = ^@; old-swtch = ^Z; susp = ^@
    [other stty output snipped]
acro% /bin/ls /bin |less &
[1] 18444 18445
acro%
[1]  + suspended (tty output)  /bin/ls /bin | less
acro% fg
[1]  + continued  /bin/ls /bin | less
4d
4d60
CC
DCC
NCC
OCC
X11
[
acroread
alpq
apropos
ar
as
at
autopush
awk
banner
basename
batch
bc
bdiff
bfs
cal
[less pauses after one page-full; I hit 'q']
acro% stty -a
speed 38400 baud; line = 1;
intr = ^C; quit = ^@; erase = DEL; kill = ^U; eof = ^A; eol = ^@; old-swtch = ^@; susp = ^@
[other stty output snipped]

------------------------------------------------------------------------------
4. vi-compatibility ZLE bug
In vi-mode, when hitting <u> for undo, the cursor does not end up
where it would in vi.

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 6,  3:59pm, Huy Le wrote:
} Subject: Remaining zsh3.0-pre2 bugs
}
} Here are some bugs that I've reported but still remain in the latest
} version of zsh.
} The apply for both HP-UX 9.01 and IRIX 5.3
} 
} 3. tty bug #2
} When tty output of "less" or "more" is suspended because it's in the
} background, after "fg"ing my tty settings are reset to default.
} This problem is hard because on HP-UX it is intermittent; happens more
} regularly on IRIX. [...]
} 
} acro% stty -a
} speed 38400 baud; line = 1;
} intr = ^C; quit = ^@; erase = DEL; kill = ^U; eof = ^D; eol = ^@; old-swtch = ^Z; susp = ^@
}     [other stty output snipped]
} [less pauses after one page-full; I hit 'q']
} acro% stty -a
} speed 38400 baud; line = 1;
} intr = ^C; quit = ^@; erase = DEL; kill = ^U; eof = ^A; eol = ^@; old-swtch = ^@; susp = ^@
} [other stty output snipped]

I've described this problem before, and the solution as well.  If
you look at <termio.h> or <termios.h> for the indices into the c_cc[]
array in the struct termio or termios, you'll discover that some of
the array positions are overloaded.  In particular, VEOF is the same
as VMIN, and VEOL is the same as VTIME.  Thus you can't use the same
struct termios to set/get the VMIN and VTIME values as well as the
TTY driver characters; you have to set and get VMIN and VTIME with
a separate structure and a separate ioctl() or tcsetattr() call.

The rule is, when (c_lflag & ICANON) then c_cc[VEOF] and c_cc[VEOL]
are valid, otherwise c_cc[VMIN] and c_cc[VTIME] are valid.

Zsh is still trying to get by with a single struct termios, which is
OK most of the time because the global shttyinfo always has ICANON,
and functions like setcbreak() and setterm() are careful to change
only a local copy of shttyinfo when setting VMIN and VTIME, so that
the right characters are restored later.  However, this can cause a
bit of confusion when what looks like a perfectly valid operation
turns out to do something unexpected.

See if this patch fixes the above bug.

*** Src/zle_main.c.orig	Fri Jul  5 10:57:47 1996
--- Src/zle_main.c	Sat Jul  6 22:04:29 1996
***************
*** 276,282 ****
  #  endif
  	    r = read(SHTTY, &cc, 1);
  #  ifdef HAVE_TERMIOS_H
! 	    tcsetattr(SHTTY, TCSANOW, &ti.tio);
  #  else
  	    ioctl(SHTTY, TCSETA, &shttyinfo.tio);
  #  endif
--- 276,282 ----
  #  endif
  	    r = read(SHTTY, &cc, 1);
  #  ifdef HAVE_TERMIOS_H
! 	    tcsetattr(SHTTY, TCSANOW, &shttyinfo.tio);
  #  else
  	    ioctl(SHTTY, TCSETA, &shttyinfo.tio);
  #  endif

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 6,  3:59pm, Huy Le wrote:
} Subject: Remaining zsh3.0-pre2 bugs
}
} 1. History bug:
} The magic-space binding seems to mess up the history of
} a line with a quoted argument that spawns two lines.

This is not a magic-space bug, specifically.  It happens any time
doexpandhist() is called on the second or later line of a command
that spans multiple lines.

} pride% echo 'a
} > b' c
} a
} b c
} pride% history
}     1  bindkey ' ' magic-space
}     2  history
}     3   c
}     4  history

But you can get the same thing with:

zsh[83] echo 'a
> b' c<TAB><RET>	<-- `c' menu-completes to `co'
a
b co
zsh[84] history -2
   82  history
   83   co
   84  history -2

Here's an even worse case; the command vanishes entirely:

zsh[85] echo 'a
> b'<TAB><RET>		<-- TAB moves cursor to beginning of line, then RET
a
b
zsh[85] history -4	<-- Note event number in prompt hasn't changed!
   81   c
   82  history
   83   co
   84  history -2
   85  history -4

I think the problem is with the "if (!err)" block near the end of
doexpandhist(), because it has a comment about resetting the cursor
to the beginning of line for vi mode.  However, I don't know enough
about how history works to want to mess with it.

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 6,  3:59pm, Huy Le wrote:
} Subject: Remaining zsh3.0-pre2 bugs
}
} 4. vi-compatibility ZLE bug
} In vi-mode, when hitting <u> for undo, the cursor does not end up
} where it would in vi.

I think this has to do with the very last line of undo() in zle_misc.c:

    cs = ue->cs;

Unless I'm mistaken, this resets the cursor position to where it was
before the undo.  Vi doesn't act that way.  However, there doesn't
seem to be any way to tell whether undo() was called from emacs-mode
"undo" or from vi-mode "vi-undo-change".  Probably the right thing is
to skip that assignment when called as vi-undo-change, but I'm not
sure that's quite right either.

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 6, 10:53pm, Bart Schaefer wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} On Jul 6,  3:59pm, Huy Le wrote:
} } Subject: Remaining zsh3.0-pre2 bugs
} }
} } 1. History bug:
} } The magic-space binding seems to mess up the history of
} } a line with a quoted argument that spawns two lines.
} 
} This is not a magic-space bug, specifically.  It happens any time
} doexpandhist() is called on the second or later line of a command
} that spans multiple lines.
} 
} zsh[85] echo 'a
} > b'<TAB><RET>	<-- TAB moves cursor to beginning of line, then RET
} a
} b
} zsh[85] history -4	<-- Note event number in prompt hasn't changed!
}    81   c
}    82  history
}    83   co
}    84  history -2
}    85  history -4
} 
} I think the problem is with the "if (!err)" block near the end of
} doexpandhist(), because it has a comment about resetting the cursor
} to the beginning of line for vi mode.  However, I don't know enough
} about how history works to want to mess with it.

Further investigation indicates that docomplete() itself is also
broken, in this section:

    /* If we are completing in a multi-line buffer (which was not taken *
     * from the history), we have to prepend the stuff saved in chline  *
     * to the contents of line.                                         */

    if (!isfirstln && chline != NULL) {
	char *p, *q;

	/* .... */
    }
    else
	ol = NULL;

I think the problem has to do with metafication of the stuff that was
in chline; specifically, here:

        /* Finally remove quoted newlines and replace non-quoted newlines *
         * with semicolons.  This second thing isn't really correct (e.g. *
         * if the command separated by the newlines were actually         *
         * connected with a pipe), but the effect in the completion code  *
         * is the same.                                                   */
        for (p = (char *)line; *p; p++)
            if (*p == '\n')
                if (p > (char *) line && p[-1] == '\\')
                    p[-1] = *p = ' ';
                else
                    *p = ';';

When that loop starts, line is "echo 'a\nb'" and after the loop, it's
"echo 'a;b'" which I'm pretty certain is wrong.  However, I don't know
what to do about it.

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Zefram]

>I think this has to do with the very last line of undo() in zle_misc.c:
>
>    cs = ue->cs;
>
>Unless I'm mistaken, this resets the cursor position to where it was
>before the undo.  Vi doesn't act that way.  However, there doesn't
>seem to be any way to tell whether undo() was called from emacs-mode
>"undo" or from vi-mode "vi-undo-change".  Probably the right thing is
>to skip that assignment when called as vi-undo-change, but I'm not
>sure that's quite right either.

That would be worse, I think.  I have plans to improve some of the
remaining vi mode bindings, including this one, but there are more
serious problems than this.  vi cut buffers, for example, basically
don't work at the moment.

-zefram

Re: Remaining zsh3.0-pre2 bugs

[Zoltan Hidvegi]

> 1. History bug:
> The magic-space binding seems to mess up the history of
> a line with a quoted argument that spawns two lines.
> Note that in the following examples the number of spaces
> and the placement of the single-quote matter.
[...]
> pride% exec ./zsh -f
> pride% bindkey ' ' magic-space
> pride% echo 'a
> > b c'
> a
> b c
> pride% history
>     1  bindkey ' ' magic-space
>     2  history

And an other bug is that here documents do not appear in the history.
That's in fact a much more difficult bug.  The bug described above can be
fixed just by adding hwgetword to the list of saved variables in lexsave.

The patch below should fix both problems.  After this patch

% cat << EOF ; echo 'foo
bar'
foobar
EOF

gives

foobar
foo
bar

which is the way other shells behave.  There is an other change in this
patch to make par_redir even more simple.  History expansion is not
disables in the name of here document (but it is still disabled in here
documents of course).  So

% cat << !$
foo
!$

will no longer work if history expansion is enabled.  Note that this change
only affect interactive shell where here documents are rarely used and even
if one uses a here document it is very unlikely that she puts a bangchar in
the end-mark.

The patch is quite big so please test it extensively as there may be some
cases when it does not behave the way it should.

Zoltan


*** Src/globals.h	1996/07/01 20:59:36	2.23
--- Src/globals.h	1996/07/01 20:59:36
***************
*** 107,112 ****
--- 107,114 ----
  /* lexical analyzer error flag */
   
  EXTERN int lexstop;
+ 
+ EXTERN struct heredocs *hdocs;
   
  /* suppress error messages */
   
*** Src/zsh.h	1996/07/05 20:50:29	2.27
--- Src/zsh.h	1996/07/05 20:50:29
***************
*** 601,606 ****
--- 601,610 ----
      int cmdoutval;
  };
  
+ struct heredocs {
+     struct heredocs *next;
+     Redir rd;
+ };
  
  /*******************************/
  /* Definitions for Hash Tables */
*** Src/parse.c	1996/07/01 20:59:36	2.15
--- Src/parse.c	1996/07/08 00:19:37
***************
*** 1118,1127 ****
  void
  par_redir(LinkList l)
  {
-     char *toks;
      struct redir *fn = (struct redir *)allocnode(N_REDIR);
      int oldcmdpos, oldnc;
-     int nohist;
  
      oldcmdpos = incmdpos;
      incmdpos = 0;
--- 1118,1125 ----
***************
*** 1130,1210 ****
  	nocorrect = 1;
      fn->type = redirtab[tok - OUTANG];
      fn->fd1 = tokfd;
-     if ((nohist = fn->type == HEREDOC || fn->type == HEREDOCDASH))
- 	STOPHIST
      yylex();
-     if (nohist)
- 	ALLOWHIST
      if (tok != STRING && tok != ENVSTRING)
  	YYERRORV;
-     toks = tokstr;
      incmdpos = oldcmdpos;
      nocorrect = oldnc;
-     yylex();
- 
- /* assign default fd */
  
      if (fn->fd1 == -1)
  	fn->fd1 = IS_READFD(fn->type) ? 0 : 1;
  
!     fn->name = toks;
  
      switch (fn->type) {
      case HEREDOC:
      case HEREDOCDASH: {
  	/* <<[-] name */
! 	char tbuf[256], *tlin = NULL;
! 	int tsiz = 0, redirl;
  
! 	/* Save the rest of the current line for later tokenization */
! 	if (!isnewlin) {
! 	    while (ingets(tbuf, 256) != NULL) {
! 		redirl = strlen(tbuf);
! 		if (tsiz == 0) {
! 		    tlin = ztrdup(tbuf);	/* Test for failure? */
! 		    tsiz = redirl;
! 		} else {
! 		    tlin = realloc(tlin, tsiz + redirl + 1);	/* Test for failure? */
! 		    strcpy(&tlin[tsiz], tbuf);
! 		    tsiz += redirl;
! 		}
! 		if (tbuf[redirl - 1] == '\n')
! 		    break;
! 	    }
! 	}
! 	cmdpush(fn->type == HEREDOC ? CS_HEREDOC : CS_HEREDOCD);
! 	/* Now grab the document */
! 	fn->name = gethere(toks, fn->type);
! 	fn->type = HERESTR;
! 	cmdpop();
! 	/* Put back the saved line to resume tokenizing */
! 	if (tsiz > 0) {
! 	    inputsetline(tlin, INP_FREE);
! 	}
  	break;
      }
- 
      case WRITE:
      case WRITENOW:
! 	if (toks[0] == Outang && toks[1] == Inpar)
  	    /* > >(...) */
  	    fn->type = OUTPIPE;
! 	else if (toks[0] == Inang && toks[1] == Inpar)
  	    YYERRORV;
  	break;
- 
      case READ:
! 	if (toks[0] == Inang && toks[1] == Inpar)
  	    /* < <(...) */
  	    fn->type = INPIPE;
! 	else if (toks[0] == Outang && toks[1] == Inpar)
  	    YYERRORV;
  	break;
      case READWRITE:
! 	if ((toks[0] == Inang || toks[0] == Outang) && toks[1] == Inpar)
! 	    fn->type = toks[0] == Inang ? INPIPE : OUTPIPE;
  	break;
      }
      addlinknode(l, fn);
  }
  
--- 1128,1178 ----
  	nocorrect = 1;
      fn->type = redirtab[tok - OUTANG];
      fn->fd1 = tokfd;
      yylex();
      if (tok != STRING && tok != ENVSTRING)
  	YYERRORV;
      incmdpos = oldcmdpos;
      nocorrect = oldnc;
  
+     /* assign default fd */
      if (fn->fd1 == -1)
  	fn->fd1 = IS_READFD(fn->type) ? 0 : 1;
  
!     fn->name = tokstr;
  
      switch (fn->type) {
      case HEREDOC:
      case HEREDOCDASH: {
  	/* <<[-] name */
! 	struct heredocs **hd;
  
! 	for (hd = &hdocs; *hd; hd = &(*hd)->next);
! 	*hd = zalloc(sizeof(struct heredocs));
! 	(*hd)->next = NULL;
! 	(*hd)->rd = fn;
  	break;
      }
      case WRITE:
      case WRITENOW:
! 	if (tokstr[0] == Outang && tokstr[1] == Inpar)
  	    /* > >(...) */
  	    fn->type = OUTPIPE;
! 	else if (tokstr[0] == Inang && tokstr[1] == Inpar)
  	    YYERRORV;
  	break;
      case READ:
! 	if (tokstr[0] == Inang && tokstr[1] == Inpar)
  	    /* < <(...) */
  	    fn->type = INPIPE;
! 	else if (tokstr[0] == Outang && tokstr[1] == Inpar)
  	    YYERRORV;
  	break;
      case READWRITE:
! 	if ((tokstr[0] == Inang || tokstr[0] == Outang) && tokstr[1] == Inpar)
! 	    fn->type = tokstr[0] == Inang ? INPIPE : OUTPIPE;
  	break;
      }
+     yylex();
      addlinknode(l, fn);
  }
  
*** Src/init.c	1996/07/07 16:03:48	2.24
--- Src/init.c	1996/07/08 00:06:51
***************
*** 737,744 ****
  {
      if (cmd) {
  	if (SHIN >= 10)
! 	    zclose(SHIN);
  	SHIN = movefd(open("/dev/null", O_RDONLY));
  	execstring(cmd, 0, 1);
  	stopmsg = 1;
  	zexit(lastval, 0);
--- 737,745 ----
  {
      if (cmd) {
  	if (SHIN >= 10)
! 	    fclose(bshin);
  	SHIN = movefd(open("/dev/null", O_RDONLY));
+ 	bshin = fdopen(SHIN, "r");
  	execstring(cmd, 0, 1);
  	stopmsg = 1;
  	zexit(lastval, 0);
*** Src/input.c	1996/06/28 02:05:24	2.7
--- Src/input.c	1996/07/07 23:05:35
***************
*** 303,324 ****
      return 0;
  }
  
- /* Read one line of at most n-1 chars from the input queue */
- 
- /**/
- char *
- ingets(char *buf, int n)
- {
-     int l;
- 
-     for (l = 0; l < n - 1 && (buf[l++] = ingetc()) != '\n' && !lexstop;);
-     if (lexstop)
- 	l--;
-     buf[l] = '\0';
- 
-     return (!lexstop || l) ? buf : NULL;
- }
- 
  /*
   * Put a string in the input queue:
   * inbuf is only freeable if the flags include INP_FREE.
--- 303,308 ----
*** Src/lex.c	1996/07/07 00:18:17	2.31
--- Src/lex.c	1996/07/07 23:36:51
***************
*** 36,41 ****
--- 36,42 ----
  static int dbparens;
  int len = 0, bsiz = 256;
  char *bptr;
+ extern int hwgetword;
  
  struct lexstack {
      struct lexstack *next;
***************
*** 55,66 ****
--- 56,70 ----
      char *hline;
      char *hptr;
      int tok;
+     int isnewlin;
      char *tokstr;
      char *bptr;
      int bsiz;
      short *chwords;
      int chwordlen;
      int chwordpos;
+     int hwgetword;
+     struct heredocs *hdocs;
  
      unsigned char *cstack;
      int csp;
***************
*** 103,116 ****
--- 107,124 ----
      ls->csp = cmdsp;
      cmdstack = (unsigned char *)zalloc(256);
      ls->tok = tok;
+     ls->isnewlin = isnewlin;
      ls->tokstr = tokstr;
      ls->bptr = bptr;
      ls->bsiz = bsiz;
      ls->chwords = chwords;
      ls->chwordlen = chwordlen;
      ls->chwordpos = chwordpos;
+     ls->hwgetword = hwgetword;
+     ls->hdocs = hdocs;
      cmdsp = 0;
      inredir = 0;
+     hdocs = NULL;
  
      ls->next = lstack;
      lstack = ls;
***************
*** 146,157 ****
--- 154,168 ----
      cmdstack = lstack->cstack;
      cmdsp = lstack->csp;
      tok = lstack->tok;
+     isnewlin = lstack->isnewlin;
      tokstr = lstack->tokstr;
      bptr = lstack->bptr;
      bsiz = lstack->bsiz;
      chwords = lstack->chwords;
      chwordlen = lstack->chwordlen;
      chwordpos = lstack->chwordpos;
+     hwgetword = lstack->hwgetword;
+     hdocs = lstack->hdocs;
      clearalstack();
      hlinesz = lstack->hlinesz;
      lexstop = errflag = 0;
***************
*** 170,175 ****
--- 181,202 ----
      do
  	tok = gettok();
      while (tok != ENDINPUT && exalias());
+     if (tok == NEWLIN || tok == ENDINPUT) {
+ 	while (hdocs) {
+ 	    struct heredocs *next = hdocs->next;
+ 
+ 	    hwbegin(0);
+ 	    cmdpush(hdocs->rd->type == HEREDOC ? CS_HEREDOC : CS_HEREDOCD);
+ 	    STOPHIST
+ 	    hdocs->rd->name = gethere(hdocs->rd->name, hdocs->rd->type);
+ 	    ALLOWHIST
+ 	    cmdpop();
+ 	    hwend();
+ 	    hdocs->rd->type = HERESTR;
+ 	    zfree(hdocs, sizeof(struct heredocs));
+ 	    hdocs = next;
+ 	}
+     }
      if (tok != NEWLIN)
  	isnewlin = 0;
      else
*** Src/hist.c	1996/07/04 19:40:08	2.16
--- Src/hist.c	1996/07/07 19:41:00
***************
*** 144,149 ****
--- 144,165 ----
      return c;
  }
  
+ /* Read one line of at most n-1 chars from the input */
+ 
+ /**/
+ char *
+ hgets(char *buf, int n)
+ {
+     int l;
+ 
+     for (l = 0; l < n - 1 && (buf[l++] = hgetc()) != '\n' && !lexstop;);
+     if (lexstop)
+ 	l--;
+     buf[l] = '\0';
+ 
+     return (!lexstop || l) ? buf : NULL;
+ }
+ 
  /**/
  void
  safeinungetc(int c)
***************
*** 725,731 ****
  }
  
  /* Gives current expansion word if not last word before chwordpos. */
! static int hwgetword = -1;
  
  /* begin a word */
  
--- 741,747 ----
  }
  
  /* Gives current expansion word if not last word before chwordpos. */
! int hwgetword = -1;
  
  /* begin a word */
  
***************
*** 794,809 ****
  {
      int pos = hwgetword > -1 ? hwgetword : chwordpos - 2;
  
! #if 0
      /* debugging only */
      if (hwgetword == -1 && !chwordpos) {
  	/* no words available */
! 	zerr("hwget called with no words.", NULL, 0);
  	*startptr = "";
  	return;
      } 
      else if (hwgetword == -1 && chwordpos%2) {
! 	zerr("hwget called in middle of word.", NULL, 0);
  	*startptr = "";
  	return;
      }
--- 810,825 ----
  {
      int pos = hwgetword > -1 ? hwgetword : chwordpos - 2;
  
! #ifdef DEBUG
      /* debugging only */
      if (hwgetword == -1 && !chwordpos) {
  	/* no words available */
! 	DPUTS(1, "hwget called with no words.");
  	*startptr = "";
  	return;
      } 
      else if (hwgetword == -1 && chwordpos%2) {
! 	DPUTS(1, "hwget called in middle of word.");
  	*startptr = "";
  	return;
      }
*** Src/exec.c	1996/07/04 19:40:08	2.53
--- Src/exec.c	1996/07/08 00:10:03
***************
*** 1442,1447 ****
--- 1442,1449 ----
      /* Do io redirections */
      while (nonempty(cmd->redir)) {
  	fn = (Redir) ugetnode(cmd->redir);
+ 	DPUTS(fn->type == HEREDOC || fn->type == HEREDOCDASH,
+ 	      "BUG: unexpanded here document\n");
  	if (fn->type == INPIPE) {
  	    if (fn->fd2 == -1) {
  		fixfds(save);
***************
*** 1852,1858 ****
      for (;;) {
  	char *u, *v;
  
! 	if (!ingets(pbuf, sizeof(pbuf)))
  	    break;
  	bptr = pbuf;
  	if (!siz || t[siz-1] == '\n') {
--- 1854,1860 ----
      for (;;) {
  	char *u, *v;
  
! 	if (!hgets(pbuf, sizeof(pbuf)))
  	    break;
  	bptr = pbuf;
  	if (!siz || t[siz-1] == '\n') {
***************
*** 1862,1868 ****
  	    for (u = bptr, v = str; *u != '\n' && *v; u++, v++)
  		if (*u != *v)
  		    break;
! 	    if (*u == '\n' && !*v)
  		break;
  	}
  	l = strlen(bptr);
--- 1864,1870 ----
  	    for (u = bptr, v = str; *u != '\n' && *v; u++, v++)
  		if (*u != *v)
  		    break;
! 	    if ((!*u || *u == '\n') && !*v)
  		break;
  	}
  	l = strlen(bptr);

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 8,  2:48am, Zoltan Hidvegi wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} And an other bug is that here documents do not appear in the history.
} That's in fact a much more difficult bug.  The bug described above can be
} fixed just by adding hwgetword to the list of saved variables in lexsave.

That doesn't fix the problem of the cursor ending up in the wrong place
following the completion, though.

} The patch below should fix both problems.
}
[parts of patch deleted]
} 
} *** Src/input.c	1996/06/28 02:05:24	2.7
} --- Src/input.c	1996/07/07 23:05:35
} ***************
} *** 303,324 ****
} - /* Read one line of at most n-1 chars from the input queue */
} - 
} - /**/
} - char *
} - ingets(char *buf, int n)
} - {
} - }
} *** Src/hist.c	1996/07/04 19:40:08	2.16
} --- Src/hist.c	1996/07/07 19:41:00
} ***************
} *** 144,149 ****
} --- 144,165 ----
} + /* Read one line of at most n-1 chars from the input */
} + 
} + /**/
} + char *
} + hgets(char *buf, int n)
} + {
} + }

This is simply too amusing.  I originally added hgets() to hist.c back
sometime late in v2.2 or early in v2.3; it then got moved to input.c,
renamed ingets(), and changed to use ingetc(), sometime late in v2.4;
and now we're back to hgets()/hgetc() again.

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 7,  9:22pm, Bart Schaefer wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} On Jul 8,  2:48am, Zoltan Hidvegi wrote:
} } Subject: Re: Remaining zsh3.0-pre2 bugs
} }
} } And an other bug is that here documents do not appear in the history.
} } That's in fact a much more difficult bug.  The bug described above can be
} } fixed just by adding hwgetword to the list of saved variables in lexsave.
} 
} That doesn't fix the problem of the cursor ending up in the wrong place
} following the completion, though.

I'm now convinced that get_comp_string() is messing up the cursor position,
somewhere in this loop:

        /* While building the quoted form, we also clean up the command line. */
        offs = cs - wb;
        for (p = s, tt = qword, i = wb; *p; p++, tt++, i++)
            if (INULL(*p)) {
                if (i < cs)
                    offs--;
                if (p[1] || *p != Bnull) {
		    /* ... */
                } else {
                    ocs = cs;
                    *tt = '\0';
                    cs = we;
                    backdel(1);
                    if (ocs == we)
                        cs = we - 1;
                    else
                        cs = ocs;
                    we--;
                }
                chuck(p--);
            }

The specific clause

                    if (ocs == we)
                        cs = we - 1;

is hit twice during this completion:

	zsh% echo 'a
	> b'<TAB>

This in turn happens because qword (which came from s, which points to
clwords[clwpos]) points to "\230a;b\230x" rather than to "'a;b'x" and
because INULL('\230') is true.

So I rather suspect that has something to do with the bug, but I'm lost
at this point in terms of knowing what \230 represents, or how it got
into tokstr and thus ultimately into clwords[1], or whether INULL() is
supposed to be true at that point.

Anyway, cs decrements down to 8 because of the two \230 in qword; but
"echo 'a;b'" of course has 10 characters.  Back in docomplete(), this
section:

    /* If we added chline to the line buffer, reset the original contents. */
    if (ol) {
        strcpy((char *) line, ol);
        ll = strlen((char *) line);
        cs -= chl;
        wb -= chl;
        we -= chl;
        if (wb < 0 || we < 0) {
            unmetafy_line();
            return;
        }
    }

then decrements cs by chl (the length of "echo 'a;"), thus cs == 0 when
it should have remained cs == 2.

Hopefully that's enough for somebody more versed in the lexer to fix
get_comp_string() ...

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 8,  2:48am, Zoltan Hidvegi wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} The patch is quite big so please test it extensively as there may be some
} cases when it does not behave the way it should.

Well, tell me if this is "as it should" or not ... with this as the current
input:

zigzag<1> echo 'a
b'

Type send-break (ctrl-G).  Now note:

zigzag<2> history
    1  echo 'a
    2  history

Should the first line of the never-executed command *really* have been put
into the history at that point?

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Zefram]

>So I rather suspect that has something to do with the bug, but I'm lost
>at this point in terms of knowing what \230 represents, or how it got
>into tokstr and thus ultimately into clwords[1], or whether INULL() is
>supposed to be true at that point.

'\230' is Snull -- the token that the ' turns into.  INULL() *is*
supposed to be true for this token -- it indicates that that character
should be ignored when considering the line at later stages of
execution.  The four tokens it is true for are Snull (single quote as
here), Dnull (double quote), Bnull (backslash) and Nularg (an empty
argument).

Note that the ; in the middle did *not* turn into a token -- the lexer
sees that it is quoted, so leaves it literal.  If the line were
actually being executed (pretending that it really is a ; and not a
newline), the code after the lexer would see an argument a;b, ignoring
the Snull tokens.  Had the quotes not been there, the ; would have
turned into a token and been treated specially.

It's really quite an elegant system.  Unfortunately it does fall over
somewhat when the token values are appearing in the input as normal
characters, but as you know we now have a workaround for that too.

>Anyway, cs decrements down to 8 because of the two \230 in qword; but
>"echo 'a;b'" of course has 10 characters.

Bingo.  That loop has special code for handling backslashes, but not
quotes.  I think it needs to handle quotes too.  The completion code
really doesn't cope with quotes at all, so I'm not surprised this is
happening.

-zefram

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 8,  8:57am, Zefram wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} '\230' is Snull -- the token that the ' turns into.  INULL() *is*
} supposed to be true for this token -- it indicates that that character
} should be ignored when considering the line at later stages of
} execution.  The four tokens it is true for are Snull (single quote as
} here), Dnull (double quote), Bnull (backslash) and Nularg (an empty
} argument).

Ah; I wondered.  I couldn't figure out whether Bnull was backslash
or backquote (as in command substitution); it's mentioned in relation
to `...` expressions in several comments.

} >Anyway, cs decrements down to 8 because of the two \230 in qword; but
} >"echo 'a;b'" of course has 10 characters.
} 
} Bingo.  That loop has special code for handling backslashes, but not
} quotes.  I think it needs to handle quotes too.

What I want to know is, why is it considered "cleaning up" the command
line to delete everything except backslashes?  Which is what that loop
seems to be doing ... my guess is that it has something to do with this:

zagzig<3> touch a' 'bfoo
zagzig<4> echo a' 'b<TAB>
zagzig<4> echo a\ bfoo

But that doesn't work for completions with embedded newlines, even when
a successful completion is possible:

zagzig<4> touch c'
> 'dfoo
zagzig<5> echo c'
> 'd<TAB>			<-- fails to complete

I suspect it fails because newline is the only character that can never
be escaped with a backslash.

And it's broken even for completions with spaces in some circumstances:

zagzig<6> echo a' '<TAB>
zagzig<6> echo a\ bfo o		<-- cursor is over the second `o'


-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Zoltan Hidvegi]

> This is simply too amusing.  I originally added hgets() to hist.c back
> sometime late in v2.2 or early in v2.3; it then got moved to input.c,
> renamed ingets(), and changed to use ingetc(), sometime late in v2.4;
> and now we're back to hgets()/hgetc() again.

Not really.  The hgets -> ingets change happened last august.  And hgets
used hgetch instead of hgetc.  The real change compared to the 2.5 version
is using hgetc instead of hgetch.  The difference is that hgetch has
nothing to do with the history code while hgetc updates chline.  So the
name ingets was correct.  But now I'll perhaps completely remove hgets and
merge it into gethere since it is only called from there and it seems that
the code would be simpler if hgets were merged into gethere.

Zoltan

Re: Remaining zsh3.0-pre2 bugs

[Zoltan Hidvegi]

Bart and Zefram discussing a loop in get_comp_string:
> } >Anyway, cs decrements down to 8 because of the two \230 in qword; but
> } >"echo 'a;b'" of course has 10 characters.
> } 
> } Bingo.  That loop has special code for handling backslashes, but not
> } quotes.  I think it needs to handle quotes too.

There is nothing wrong with get_comp_string here.  It works well if there
is just one line containing embeded newlines and quotes etc.  The problem
starts when we are in a multi-line command structure.

[...]
> But that doesn't work for completions with embedded newlines, even when
> a successful completion is possible:
> 
> zagzig<4> touch c'
> > 'dfoo
> zagzig<5> echo c'
> > 'd<TAB>			<-- fails to complete
> 
> I suspect it fails because newline is the only character that can never
> be escaped with a backslash.

It fails because zsh sees that the current word starts in the previous
already entered line.  In that case zsh simply gives up the completion
attempt, restores the original line and returns.  Unfortunately the cursor
position is not always restored correctly.  The patch below fixes that.

The other problem is this:

hzoli /l/src/zsh-RCS % echo '
quote> ' a' 'b<TAB>

hzoli /l/src/zsh-RCS % echo '
quote> ' a\ bfoo 'b            <-- cursor is under the quote preceeding `b'

That is because the changes made by get_comp_string are destroyed.

> And it's broken even for completions with spaces in some circumstances:
> 
> zagzig<6> echo a' '<TAB>
> zagzig<6> echo a\ bfo o		<-- cursor is over the second `o'

And that's a third problem caused by that code:

-     /* When completing between words, the contents *
-      * of wb and we may be garbled.                */
-     if (we > wb && iblank(line[we - 1]) && 
- 	(we < 2 || (line[we - 2] != '\\' && line[we - 2] != STOUC(Meta))))
- 	we--;

I do not know what it meant to fix but I hope it is no longer necessary.
This should be handled in get_comp_string.  get_comp_string changed a lot
since 2.5 so I think it's best to remove this.

And there is one more bug when push-line-or-edit is used:

hzoli /l/src/zsh-RCS % echo '
quote>'<push-line-or-edit>
hwget called with no words.
hzoli /l/src/zsh-RCS % echo '
'

The debug message above only appears after my recent here-document patch.
That is fixed by the one-line patch to lex.c below.

Zoltan


*** Src/zle_tricky.c	1996/07/06 18:17:13	2.56
--- Src/zle_tricky.c	1996/07/09 01:00:32
***************
*** 497,503 ****
  docomplete(int lst)
  {
      char *s, *ol;
!     int olst = lst, chl = 0, ne = noerrs;
  
      /* If we are doing a menu-completion... */
  
--- 497,503 ----
  docomplete(int lst)
  {
      char *s, *ol;
!     int olst = lst, chl = 0, ne = noerrs, ocs;
  
      /* If we are doing a menu-completion... */
  
***************
*** 529,534 ****
--- 529,535 ----
  	char *p, *q;
  
  	ol = dupstring((char *)line);
+ 	ocs = cs;
  	/* Make sure that chline is zero-terminated. */
  	*hptr = '\0';
  	/* Calculate the length of chline. */
***************
*** 550,571 ****
  	    }
  	    *q++ = *p;
  	}
! 	/* Put a zero byte after it and append the original line contents. */
! 	*q = '\0';
! 	strcat((char *)line, ol);
! 	/* Finally remove quoted newlines and replace non-quoted newlines *
! 	 * with semicolons.  This second thing isn't really correct (e.g. *
! 	 * if the command separated by the newlines were actually         *
! 	 * connected with a pipe), but the effect in the completion code  *
! 	 * is the same.                                                   */
! 	for (p = (char *)line; *p; p++)
! 	    if (*p == '\n')
! 		if (p > (char *) line && p[-1] == '\\')
! 		    p[-1] = *p = ' ';
! 		else
! 		    *p = ';';
! 	/* Now make sure that the cursor position variable and the *
! 	 * line length stored in ll are correct.                   */
  	cs += chl;
  	ll = strlen((char *)line);
      }
--- 551,558 ----
  	    }
  	    *q++ = *p;
  	}
! 	/* Append the original line contents. */
! 	strcpy(q, ol);
  	cs += chl;
  	ll = strlen((char *)line);
      }
***************
*** 577,587 ****
      noerrs = 1;
      s = get_comp_string();
      noerrs = ne;
-     /* When completing between words, the contents *
-      * of wb and we may be garbled.                */
-     if (we > wb && iblank(line[we - 1]) && 
- 	(we < 2 || (line[we - 2] != '\\' && line[we - 2] != STOUC(Meta))))
- 	we--;
      /* For vi mode, reset the start-of-insertion pointer to the beginning *
       * of the word being completed, if it is currently later.  Vi itself  *
       * would never change the pointer in the middle of an insertion, but  *
--- 564,569 ----
***************
*** 591,605 ****
  	viinsbegin = ztrsub((char *) line + wb, (char *) line);
      /* If we added chline to the line buffer, reset the original contents. */
      if (ol) {
- 	strcpy((char *) line, ol);
- 	ll = strlen((char *) line);
  	cs -= chl;
  	wb -= chl;
  	we -= chl;
! 	if (wb < 0 || we < 0) {
  	    unmetafy_line();
  	    return;
  	}
      }
      freeheap();
      /* Save the lexer state, in case the completion code uses the lexer *
--- 573,592 ----
  	viinsbegin = ztrsub((char *) line + wb, (char *) line);
      /* If we added chline to the line buffer, reset the original contents. */
      if (ol) {
  	cs -= chl;
  	wb -= chl;
  	we -= chl;
! 	if (wb < 0 || we < 0 || cs < 0) {
! 	    strcpy((char *) line, ol);
! 	    ll = strlen((char *) line);
! 	    cs = ocs;
  	    unmetafy_line();
  	    return;
  	}
+ 	ocs = cs;
+ 	cs = 0;
+ 	foredel(chl);
+ 	cs = ocs;
      }
      freeheap();
      /* Save the lexer state, in case the completion code uses the lexer *
*** Src/lex.c	1996/07/08 01:56:51	2.32
--- Src/lex.c	1996/07/08 23:54:52
***************
*** 180,186 ****
  	return;
      do
  	tok = gettok();
!     while (tok != ENDINPUT && exalias());
      if (tok == NEWLIN || tok == ENDINPUT) {
  	while (hdocs) {
  	    struct heredocs *next = hdocs->next;
--- 180,186 ----
  	return;
      do
  	tok = gettok();
!     while (tok != ENDINPUT && !errflag && exalias());
      if (tok == NEWLIN || tok == ENDINPUT) {
  	while (hdocs) {
  	    struct heredocs *next = hdocs->next;

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 9,  3:32am, Zoltan Hidvegi wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} Bart and Zefram discussing a loop in get_comp_string:
} > } >Anyway, cs decrements down to 8 because of the two \230 in qword; but
} > } >"echo 'a;b'" of course has 10 characters.
} > } 
} > } Bingo.  That loop has special code for handling backslashes, but not
} > } quotes.  I think it needs to handle quotes too.
} 
} There is nothing wrong with get_comp_string here.  It works well if there
} is just one line containing embeded newlines and quotes etc.  The problem
} starts when we are in a multi-line command structure.

Er, yeah, but that seems like an odd definition of "works".

} > But that doesn't work for completions with embedded newlines, even when
} > a successful completion is possible:
} 
} It fails because zsh sees that the current word starts in the previous
} already entered line.

Where's that happening?  Not in get_comp_string() ...

} In that case zsh simply gives up the completion
} attempt, restores the original line and returns.

Why is that necessary?  Maybe it's necessary if the word *ends* in the
previous line for some reason, but ...

} Unfortunately the cursor
} position is not always restored correctly.  The patch below fixes that.

Hmm.  Your patch does a couple of things.  One, it removes the loop that
folds embedded newlines into semicolons; two, it attempts to save and
restore the cursor position.  There's one bogus compiler warning:

zle_tricky.c: In function `docomplete':
zle_tricky.c:500: warning: `ocs' might be used uninitialized in this function

I cleared this by initializing ocs = 0 in the declaration.

*** Src/zle_tricky.c.orig	1996/07/09 00:50:55 1996
--- Src/zle_tricky.c	1996/07/09 01:00:32 1996
***************
*** 497,503 ****
  docomplete(int lst)
  {
      char *s, *ol;
!     int olst = lst, chl = 0, ne = noerrs, ocs;
  
      /* If we are doing a menu-completion... */
  
--- 497,503 ----
  docomplete(int lst)
  {
      char *s, *ol;
!     int olst = lst, chl = 0, ne = noerrs, ocs = 0;
  
      /* If we are doing a menu-completion... */
  
***************

By the way, there are block-locals named ocs and ol used in various
places in docomplete(), which could lead to confusion.  I didn't try
to fix that, though.

Anyway, after the newlines-to-semicolons thing is removed, it doesn't
seem to make any difference whether (wb < 0 || we < 0 || cs < 0).  I
commented out that entire if(){} block, and was then able to complete
file names containing embedded newlines just fine.  So do you see any
problem with the following patch, which moves that block to after the
completion is attempted (thus fixing up failure cases)?

*** Src/zle_tricky.c.orig	Tue Jul  9 00:50:55 1996
--- Src/zle_tricky.c	Tue Jul  9 01:58:06 1996
***************
*** 573,592 ****
  	viinsbegin = ztrsub((char *) line + wb, (char *) line);
      /* If we added chline to the line buffer, reset the original contents. */
      if (ol) {
! 	cs -= chl;
  	wb -= chl;
  	we -= chl;
- 	if (wb < 0 || we < 0 || cs < 0) {
- 	    strcpy((char *) line, ol);
- 	    ll = strlen((char *) line);
- 	    cs = ocs;
- 	    unmetafy_line();
- 	    return;
- 	}
- 	ocs = cs;
  	cs = 0;
  	foredel(chl);
! 	cs = ocs;
      }
      freeheap();
      /* Save the lexer state, in case the completion code uses the lexer *
--- 573,584 ----
  	viinsbegin = ztrsub((char *) line + wb, (char *) line);
      /* If we added chline to the line buffer, reset the original contents. */
      if (ol) {
! 	int tcs = (cs -= chl);
  	wb -= chl;
  	we -= chl;
  	cs = 0;
  	foredel(chl);
! 	cs = tcs;
      }
      freeheap();
      /* Save the lexer state, in case the completion code uses the lexer *
***************
*** 740,745 ****
--- 732,742 ----
  	    /* Just do completion. */
  	    docompletion(s, lst, lincmd);
  	zsfree(s);
+     }
+     if (ol && !nmatches && (wb < 0 || we < 0 || cs < 0)) {
+ 	strcpy((char *) line, ol);
+ 	ll = strlen((char *) line);
+ 	cs = ocs;
      }
      /* Reset the lexer state, pop the heap. */
      lexrestore();

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Zoltan Hidvegi]

> } > But that doesn't work for completions with embedded newlines, even when
> } > a successful completion is possible:
> } 
> } It fails because zsh sees that the current word starts in the previous
> } already entered line.
> 
> Where's that happening?  Not in get_comp_string() ...

Certainly not.  The wb < 0 check does that what you have just deleted in the
patch you sent.

> 
> } In that case zsh simply gives up the completion
> } attempt, restores the original line and returns.
> 
> Why is that necessary?  Maybe it's necessary if the word *ends* in the
> previous line for some reason, but ...

The problem is that that:

% echo 'z'\
> l<TAB>

Here get_comp_string removes the quotes around 'z' but it does not work
since the already entered part cannot be modified.  The quotes have to be
removed for completing filenames.  This later causes a SEGV.

> Hmm.  Your patch does a couple of things.  One, it removes the loop that
> folds embedded newlines into semicolons; two, it attempts to save and

Yes, it seems that it is not necessary.  That mainly caused problems after
a backslash <newline>.  Perhaps that's the bug mentioned in Etc/BUGS.

> restore the cursor position.  There's one bogus compiler warning:
> 
> zle_tricky.c: In function `docomplete':
> zle_tricky.c:500: warning: `ocs' might be used uninitialized in this function
> 
> I cleared this by initializing ocs = 0 in the declaration.

gcc is wrong here but I agree that we have to make it happy.

> By the way, there are block-locals named ocs and ol used in various
> places in docomplete(), which could lead to confusion.  I didn't try
> to fix that, though.

I do not think that confusing.  These are just temporary variables.  And
ol/ocs always store some previous line/position so the usage is
consistent.

> Anyway, after the newlines-to-semicolons thing is removed, it doesn't
> seem to make any difference whether (wb < 0 || we < 0 || cs < 0).  I

This is still necessary and it is completely unrelated to the newlines to
semicolons change.  There was nothing wrong about converting unquited
newlines to semicolons.  It worked well.  But I removed it because I think
it is unnecessary.

Zoltan

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 9,  4:06pm, Zoltan Hidvegi wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} > } In that case zsh simply gives up the completion
} > } attempt, restores the original line and returns.
} > 
} > Why is that necessary?  Maybe it's necessary if the word *ends* in the
} > previous line for some reason, but ...
} 
} The problem is that that:
} 
} % echo 'z'\
} > l<TAB>
} 
} Here get_comp_string removes the quotes around 'z' but it does not work
} since the already entered part cannot be modified.  The quotes have to be
} removed for completing filenames.  This later causes a SEGV.

Well, then, why not catch that special case (backslash-newline) and let
other cases of embedded newlines keep working?  Patch below.

} > Hmm.  Your patch does a couple of things.  One, it removes the loop that
} > folds embedded newlines into semicolons; two, it attempts to save and
} 
} Yes, it seems that it is not necessary.  That mainly caused problems after
} a backslash <newline>.  Perhaps that's the bug mentioned in Etc/BUGS.

Could very well be that bug.  With your patches plus mine below, completion
doesn't work at all for words that cross a backslash-newline (which seems
to me to be the correct behavior), but works for words later on the line
and words that contain quoted newlines.

There's one other bug that this patch does NOT address; and maybe it's not
a bug at all.  However, here's the example:

zagzig<6> touch cegi efgh
zagzig<7> echo c'e<TAB>
zagzig<7> echo c'efgh

If I do it with TWO quotes (I don't have rcquotes set):

zagzig<8> echo c''e<TAB>
zagzig<8> echo cegi

zagzig<9> echo c'e'<TAB>
zagzig<9> echo cegi

And yet:

zagzig<10> echo c'e'<C-b><TAB>		<-- Complete inside second quote
zagzig<10> echo c'efg '			<-- Cursor ends on second quote

Seems to me that case <7> should just feep at me; I'm not sure whether
I think <10> should feep or work like <9>, or whether completeinword
should make a difference, but what it did seems wrong.

Here's the patch for backslash-newline.  It includes (rather than
follows onto) my previous patch for quoted newlines.

*** Src/zle_tricky.c.0	Tue Jul  9 00:50:55 1996
--- Src/zle_tricky.c	Wed Jul 10 12:02:51 1996
***************
*** 497,503 ****
  docomplete(int lst)
  {
      char *s, *ol;
!     int olst = lst, chl = 0, ne = noerrs, ocs = 0;
  
      /* If we are doing a menu-completion... */
  
--- 497,503 ----
  docomplete(int lst)
  {
      char *s, *ol;
!     int olst = lst, chl = 0, ne = noerrs, ocs = 0, oll = 0;
  
      /* If we are doing a menu-completion... */
  
***************
*** 554,559 ****
--- 554,560 ----
  	/* Append the original line contents. */
  	strcpy(q, ol);
  	cs += chl;
+ 	oll = ll;
  	ll = strlen((char *)line);
      }
      else
***************
*** 573,592 ****
  	viinsbegin = ztrsub((char *) line + wb, (char *) line);
      /* If we added chline to the line buffer, reset the original contents. */
      if (ol) {
! 	cs -= chl;
  	wb -= chl;
  	we -= chl;
! 	if (wb < 0 || we < 0 || cs < 0) {
  	    strcpy((char *) line, ol);
! 	    ll = strlen((char *) line);
  	    cs = ocs;
  	    unmetafy_line();
  	    return;
  	}
- 	ocs = cs;
  	cs = 0;
  	foredel(chl);
! 	cs = ocs;
      }
      freeheap();
      /* Save the lexer state, in case the completion code uses the lexer *
--- 574,594 ----
  	viinsbegin = ztrsub((char *) line + wb, (char *) line);
      /* If we added chline to the line buffer, reset the original contents. */
      if (ol) {
! 	int tcs = (cs -= chl);
  	wb -= chl;
  	we -= chl;
! 	/* If completion has to back up across a backslash-newline pair, *
! 	 * the necessary rewrites are impossible.  Give up and bail out. */
! 	if (wb < (oll - ll) || we < (oll - ll) || cs < (oll - ll)) {
  	    strcpy((char *) line, ol);
! 	    ll = oll;
  	    cs = ocs;
  	    unmetafy_line();
  	    return;
  	}
  	cs = 0;
  	foredel(chl);
! 	cs = tcs;
      }
      freeheap();
      /* Save the lexer state, in case the completion code uses the lexer *
***************
*** 741,746 ****
--- 743,753 ----
  	    docompletion(s, lst, lincmd);
  	zsfree(s);
      }
+     if (ol && !nmatches && (wb < 0 || we < 0 || cs < 0)) {
+ 	strcpy((char *) line, ol);
+ 	ll = oll;
+ 	cs = ocs;
+     }
      /* Reset the lexer state, pop the heap. */
      lexrestore();
      popheap();

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Zoltan Hidvegi]

> } > } In that case zsh simply gives up the completion
> } > } attempt, restores the original line and returns.
> } > 
> } > Why is that necessary?  Maybe it's necessary if the word *ends* in the
> } > previous line for some reason, but ...
> } 
> } The problem is that that:
> } 
> } % echo 'z'\
> } > l<TAB>
> } 
> } Here get_comp_string removes the quotes around 'z' but it does not work
> } since the already entered part cannot be modified.  The quotes have to be
> } removed for completing filenames.  This later causes a SEGV.
> 
> Well, then, why not catch that special case (backslash-newline) and let
> other cases of embedded newlines keep working?  Patch below.

It did not study your patch too much so perhaps I just do not see something
obvious but I do not understand what is this wb < (oll - ll) check?

But not only backslash-newline is the problem.  It was just an example.
Try completing after

% echo '***
> ***'

Does it work with your patch?  Here makecomplist() wants to modify the
line.  Note that my patch does not completely disables completion after a
backslash-newline.  It just disables it when the current word starts in an
already entered line.  And even in that case completion is possible after
executing push-line-or-edit.  It is seems that it's not easy to correctly
handle this special and it does not worth it since it occurs very rarely.

> There's one other bug that this patch does NOT address; and maybe it's not
> a bug at all.  However, here's the example:
> 
> zagzig<6> touch cegi efgh
> zagzig<7> echo c'e<TAB>
> zagzig<7> echo c'efgh
> 
> If I do it with TWO quotes (I don't have rcquotes set):
> 
> zagzig<8> echo c''e<TAB>
> zagzig<8> echo cegi
> 
> zagzig<9> echo c'e'<TAB>
> zagzig<9> echo cegi
> 
> And yet:
> 
> zagzig<10> echo c'e'<C-b><TAB>	<-- Complete inside second quote
> zagzig<10> echo c'efg '		<-- Cursor ends on second quote

When you inside a quoted string zsh assumes that the word begins after the
quote and ignores the part of the word before the quote.  In fact the
quotes is treated as a space in that case.

Zoltan

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 10,  9:48pm, Zoltan Hidvegi wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} > } The problem is that that:
} > } 
} > } % echo 'z'\
} > } > l<TAB>
} > } 
} > } Here get_comp_string removes the quotes around 'z' but it does not work
} > } since the already entered part cannot be modified.  The quotes have to be
} > } removed for completing filenames.  This later causes a SEGV.
} > 
} > Well, then, why not catch that special case (backslash-newline) and let
} > other cases of embedded newlines keep working?  Patch below.
} 
} It did not study your patch too much so perhaps I just do not see something
} obvious but I do not understand what is this wb < (oll - ll) check?

get_comp_string() computes a "wrong" value for wb, we, and cs when the
completed form of a word contains fewer characters than the original.
That happens with backslash-newline, because "a\\\nb" becomes "ab" when
parsed.  I may be following this wrong -- I'm not very familiar with the
completion code -- but I think the seg fault results when wb or one of
the others is decremented not just past the embedded newline (wb < 0),
but back farther than the beginning of the previous line.

} But not only backslash-newline is the problem.  It was just an example.
} Try completing after
} 
} % echo '***
} > ***'
} 
} Does it work with your patch?  Here makecomplist() wants to modify the
} line.

Define "work".  If I type tab immediately after the closing quote, it
feeps and the line is not changed.  I believe that's because of the
hunk at the end of my patch, `if (ol && !nmatches && (wb < 0 ...))',
which restores the old line after discovering that the completion failed.

However, I now see that I didn't consider the case of completion after
something unusual like:

% *\
> *

This can still end up writing to (*(line - 3)) in makecomplist().  I'm
beginning to think the problem is not with completion per se, but with
expansion (and therefore with compctls that use -U).  Also, nmatches is
never set with expansions, so my `if (ol && !nmatches ...)' test isn't
accurate if expansion is all that occurs.

} And even in that case completion is possible after
} executing push-line-or-edit.  It is seems that it's not easy to correctly
} handle this special and it does not worth it since it occurs very rarely.

OK; I give up, for now.

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: Remaining zsh3.0-pre2 bugs

[Peter Stephenson]

huyle@chop.ugcs.caltech.edu wrote:
> When running either of the two sequences:
>   while echo crap >/dev/null; do done
>   while true; do echo crap >/dev/null; done
> and interrupting them with <CTRL>-C, my output gets lost from then on.
> This is an intermittent problem on HP-UX--keep doing it and eventually
> it'll happen. On IRIX, the bug has happened to me every time.
> 
> pride% while echo crap >/dev/null; do done
> pride% echo yes
> pride% exec >/dev/tty
> pride% echo yes
> yes
> pride%

This exists not just under those systems, but under Solaris 2.5 too,
so it's probably not a system problem.  It occurs probably more than
half the time; changing > to >> doesn't help.  It's clear that if zsh
is interrupted somewhere or other the fd's don't get restored
properly.  Running it under gdb, the most common places for an
interruption are in the original open() for the redirection, or the
close() either in movefd() or in redup().  I've put in all sorts of
tests to look for bad returns from system functions like these, tried
blocking interrupts around the redirection code and fixfds(), and none
of it has done any good.

Help

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

Re: Remaining zsh3.0-pre2 bugs

[Zoltan Hidvegi]

> } It did not study your patch too much so perhaps I just do not see something
> } obvious but I do not understand what is this wb < (oll - ll) check?
> 
> get_comp_string() computes a "wrong" value for wb, we, and cs when the
> completed form of a word contains fewer characters than the original.
> That happens with backslash-newline, because "a\\\nb" becomes "ab" when
> parsed.  I may be following this wrong -- I'm not very familiar with the
> completion code -- but I think the seg fault results when wb or one of
> the others is decremented not just past the embedded newline (wb < 0),
> but back farther than the beginning of the previous line.

Not really.  If you complete after

% echo '****
> '

makecomplist() does a backdel() to replace the line to

% echo \*\*\*\*"
"

That will overwrite something before line in the memory.

> Define "work".  If I type tab immediately after the closing quote, it
> feeps and the line is not changed.  I believe that's because of the
> hunk at the end of my patch, `if (ol && !nmatches && (wb < 0 ...))',
> which restores the old line after discovering that the completion failed.

But the line should not be restored since

% echo '*'<TAB>

should give

% echo \*

and not

% echo *

I do not line you alternative solution as well which executes
push-line-or-edit because it is probably not very convinient for those
using single_line_zle.  I think it is not a big limitation that completion
does not work in this rare case.  Note that

% echo foo\
> bar zle_t<TAB>

works and gives

% echo foo\
> bar zle_tricky.c

Zoltan

Redirection bug fix

[Peter Stephenson]

I wrote:
> huyle@chop.ugcs.caltech.edu reported a bug that:
> > pride% while echo crap >/dev/null; do done
> > ^C
> > pride% echo yes
> >                       <- nothing.  stdout has disappeared.
>
> Help

Actually, it's horribly obvious.  When errflag is set after a return
from addvars(), the shell thinks it was because the addvars() failed
and returns.  However, there was no code there to restore the fd's.
(The first patch was a note to me in case I got too zealous.)

Although it's not necessary here, it might prevent further bugs to
change the `if (unset(NOEXEC)) {' test in execcmd() to 
`if (unset(NOEXEC) && !errflag) {', i.e. if there was an interrupt and
errflag was set, don't try to execute anything at all.  This can't be
wrong and may save some time as well as later blood and tears.

*** Src/exec.c.redir	Wed Jul 10 15:48:17 1996
--- Src/exec.c	Thu Jul 11 14:41:32 1996
***************
*** 1554,1559 ****
--- 1554,1563 ----
  	for (i = 0; i < 10; i++)
  	    if (save[i] != -2)
  		zclose(save[i]);
+ 	/*
+ 	 * Here we specifically *don't* restore the original fd's
+ 	 * before returning.
+ 	 */
  	return;
      }
  
***************
*** 1583,1588 ****
--- 1587,1593 ----
  		if (errflag) {
  		    restore_params(restorelist, removelist);
  		    lastval = 1;
+ 		    fixfds(save);
  		    return;
  		}
  	    }
***************
*** 1653,1658 ****
--- 1658,1664 ----
  		addvars(cmd->vars, 1);
  		if (errflag) {
  		    lastval = 1;
+ 		    fixfds(save);
  		    return;
  		}
  	    }



-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

Re: Remaining zsh3.0-pre2 bugs

[Bart Schaefer]

On Jul 11,  2:16pm, Zoltan Hidvegi wrote:
} Subject: Re: Remaining zsh3.0-pre2 bugs
}
} But the line should not be restored since
} 
} % echo '*'<TAB>
} 
} should give
} 
} % echo \*
} 
} and not
} 
} % echo *

When I say "restored", I mean *restored*.  What it gives after my patch is

% echo '*'

Not * or \*.  (Actually, I'm fibbing.  With "'*'" it would give "\\*"
because there's no embedded newline.  With "'*\n'" it gives "'*\n'".
Which is exactly what it gave before my patch, in the case where the
completion fails.)

} I do not line you alternative solution as well which executes
} push-line-or-edit because it is probably not very convinient for those
} using single_line_zle.

Well, that could be easily caught, too, and calling pushlineoredit could
be skipped in all cases if wb >= chl.  (For confused zsh-workers, I sent
one more patch only to Zoltan, which would automatically pushlineoredit
when completion may have modified something across an embedded newline.)

However, you're right that it's not that big a deal.  Could we at least
put a `feep();' before `return;' inside the `if (wb < 0 || ...)' so that
it doesn't seem that zsh is sitting there doing nothing?

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"