BUG: vared in function

BUG: vared in function

[Bernd Eggink]

Argh... 

   # t
   vared -cp "Aha: " BLAH

   % t
   t:vared:2: ZLE not enabled

This breaks lots of my scripts! 
On the command line, 'vared' works. 

Bernd

-- 
Bernd Eggink
Regionales Rechenzentrum der Uni Hamburg
eggink@uni-hamburg.de
http://www.rrz.uni-hamburg.de/eggink/BEggink.html

Re: BUG: vared in function

[Peter Stephenson]

Bernd Eggink wrote:
> Argh... 
> 
>    # t
>    vared -cp "Aha: " BLAH
> 
>    % t
>    t:vared:2: ZLE not enabled
> 
> This breaks lots of my scripts! 
> On the command line, 'vared' works. 

(I take it that should be `script' rather than `function' in the subject
line, functions are OK.)

I don't think there's any good reason why zle shouldn't be enabled
especially, if it can find a terminal which it can, but until whoever made
the change takes a look you can workaround it by giving zsh the -i option
to run the script interactively.  That might have other side effects,
though.

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@CambridgeSiliconRadio.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: BUG: vared in function

[Peter Stephenson]

Peter Stephenson wrote:
> Bernd Eggink wrote:
> > Argh... 
> > 
> >    # t
> >    vared -cp "Aha: " BLAH
> > 
> >    % t
> >    t:vared:2: ZLE not enabled
> > 
> > This breaks lots of my scripts! 
> > On the command line, 'vared' works. 
> 
> (I take it that should be `script' rather than `function' in the subject
> line, functions are OK.)
> 
> I don't think there's any good reason why zle shouldn't be enabled
> especially.

In fact, what about this?  If it works for read, shouldn't it work for
vared?  It certainly seems to.

I removed the references to oshout, because it seems to me that if SHTTY is
-1 shout has no business being anything but NULL --- that was certainly the
intention when I introduced shout, although there's been a lot of water
under the bridge (and everywhere else) since then.  Maybe the question even
got asked, in which case I missed it.

I'm in two minds on whether to commit this.  On the one hand, there is more
of a potential for problems with read or vared.  On the other hand, without
it it introduces a new problem just for one version.

I remembered the mod_export, anyway.


Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.20
diff -u -r1.20 builtin.c
--- Src/builtin.c	2000/05/27 08:31:32	1.20
+++ Src/builtin.c	2000/05/31 22:22:51
@@ -3357,6 +3357,39 @@
 static char *zbuf;
 static int readfd;
 
+/**/
+mod_export int
+openttyspecially(int kopt)
+{
+    int haso = 0;
+
+    if (SHTTY == -1) {
+	/* need to open /dev/tty specially */
+	if ((SHTTY = open("/dev/tty", O_RDWR|O_NOCTTY)) != -1) {
+	    haso = 1;
+	    init_shout();
+	}
+    } else if (!shout) {
+	/* We need an output FILE* on the tty */
+	init_shout();
+    }
+    /* We should have a SHTTY opened by now. */
+    if (SHTTY == -1) {
+	/* Unfortunately, we didn't. */
+	fprintf(stderr, "not interactive and can't open terminal\n");
+	fflush(stderr);
+	return 1;
+    }
+    if (unset(INTERACTIVE))
+	gettyinfo(&shttyinfo);
+    /* attach to the tty */
+    attachtty(mypgrp);
+    if (kopt)
+	setcbreak();
+    readfd = SHTTY;
+    return haso;
+}
+
 /* Read a character from readfd, or from the buffer zbuf.  Return EOF on end of
 file/buffer. */
 
@@ -3376,7 +3409,6 @@
     int isem = !strcmp(term, "emacs"), izle = zleactive && getkeyptr;
     char *buf, *bptr, *firstarg, *zbuforig;
     LinkList readll = newlinklist();
-    FILE *oshout = NULL;
 
     if ((ops['k'] || ops['b']) && *args && idigit(**args)) {
 	if (!(nchars = atoi(*args)))
@@ -3398,33 +3430,8 @@
 	return compctlread(name, args, ops, reply);
 
     if ((ops['k'] && !ops['u'] && !ops['p']) || ops['q']) {
-	if (!zleactive) {
-	    if (SHTTY == -1) {
-		/* need to open /dev/tty specially */
-		if ((SHTTY = open("/dev/tty", O_RDWR|O_NOCTTY)) != -1) {
-		    haso = 1;
-		    oshout = shout;
-		    init_shout();
-		}
-	    } else if (!shout) {
-		/* We need an output FILE* on the tty */
-		init_shout();
-	    }
-	    /* We should have a SHTTY opened by now. */
-	    if (SHTTY == -1) {
-		/* Unfortunately, we didn't. */
-		fprintf(stderr, "not interactive and can't open terminal\n");
-		fflush(stderr);
-		return 1;
-	    }
-	    if (unset(INTERACTIVE))
-		gettyinfo(&shttyinfo);
-	    /* attach to the tty */
-	    attachtty(mypgrp);
-	    if (!isem && ops['k'])
-		setcbreak();
-	    readfd = SHTTY;
-	}
+	if (!zleactive)
+	    haso = openttyspecially(!isem && ops['k']);
 	keys = 1;
     } else if (ops['u'] && !ops['p']) {
 	/* -u means take input from the specified file descriptor. *
@@ -3485,7 +3492,7 @@
 		settyinfo(&shttyinfo);
 	    if (haso) {
 		fclose(shout);	/* close(SHTTY) */
-		shout = oshout;
+		shout = NULL;
 		SHTTY = -1;
 	    }
 	}
@@ -3517,7 +3524,7 @@
 	    /* dispose of result appropriately, etc. */
 	    if (haso) {
 		fclose(shout);	/* close(SHTTY) */
-		shout = oshout;
+		shout = NULL;
 		SHTTY = -1;
 	    }
 	}
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.5
diff -u -r1.5 zle_main.c
--- Src/Zle/zle_main.c	2000/05/30 03:43:27	1.5
+++ Src/Zle/zle_main.c	2000/05/31 22:22:55
@@ -741,10 +741,11 @@
     int create = 0, ifl;
     int type = PM_SCALAR, obreaks = breaks, haso = 0;
     char *p1 = NULL, *p2 = NULL;
-    FILE *oshout = NULL;
 
-    if (unset(USEZLE)) {
-	zwarnnam(name, "ZLE not enabled", NULL, 0);
+    if (unset(USEZLE) || SHTTY == -1)
+	haso = openttyspecially(0);
+    if (SHTTY == -1) {
+	zwarnnam(name, "can't open terminal", NULL, 0);
 	return 1;
     }
     if (zleactive) {
@@ -830,17 +831,6 @@
 	return 1;
     }
 
-    if (SHTTY == -1) {
-	/* need to open /dev/tty specially */
-	if ((SHTTY = open("/dev/tty", O_RDWR|O_NOCTTY)) == -1) {
-	    zwarnnam(name, "can't access terminal", NULL, 0);
-	    return 1;
-	}
-	oshout = shout;
-	init_shout();
-
-	haso = 1;
-    }
     /* edit the parameter value */
     zpushnode(bufstack, ztrdup(s));
 
@@ -857,7 +847,7 @@
     varedarg = ova;
     if (haso) {
 	fclose(shout);	/* close(SHTTY) */
-	shout = oshout;
+	shout = NULL;
 	SHTTY = -1;
     }
     if (!t || errflag) {

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@CambridgeSiliconRadio.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: BUG: vared in function

[Bart Schaefer]

On May 31, 11:28pm, Peter Stephenson wrote:
> Subject: Re: BUG: vared in function
> Peter Stephenson wrote:
> > Bernd Eggink wrote:
> > > Argh... 
> > > 
> > >    # t
> > >    vared -cp "Aha: " BLAH
> > > 
> > >    % t
> > >    t:vared:2: ZLE not enabled
> > > 
> > > This breaks lots of my scripts! 

This is happening because of my bug report to the effect that vared was
improperly munging the TTY settings in an interactive shell that had ZLE
explicitly turned off (as e.g. inside and emacs shell buffer).

I don't know how that interacts with Peter's patch (I'll try it later when
I get home).

> I'm in two minds on whether to commit this.  On the one hand, there is more
> of a potential for problems with read or vared.  On the other hand, without
> it it introduces a new problem just for one version.

I think there's a bug here:

> +    /* We should have a SHTTY opened by now. */
> +    if (SHTTY == -1) {
> +	/* Unfortunately, we didn't. */
> +	fprintf(stderr, "not interactive and can't open terminal\n");
> +	fflush(stderr);
> +	return 1;
> +    }

That `return 1;' is supposed to be an abort of (what is now) the *caller*
of openttyspecially().  As you have it, that's going to make the caller
think that opening the tty succeeded.  If you're going to pull this stuff
out into a function, you need three return states, not two.

Re: BUG: vared in function

[Bart Schaefer]

On May 31,  5:00pm, Bart Schaefer wrote:
} Subject: Re: BUG: vared in function
}
} On May 31, 11:28pm, Peter Stephenson wrote:
} > Subject: Re: BUG: vared in function
} > Peter Stephenson wrote:
} > > Bernd Eggink wrote:
} > > > Argh... 
} > > > 
} > > >    # t
} > > >    vared -cp "Aha: " BLAH
} > > > 
} > > >    % t
} > > >    t:vared:2: ZLE not enabled
} > > > 
} > > > This breaks lots of my scripts! 
} 
} This is happening because of my bug report to the effect that vared was
} improperly munging the TTY settings in an interactive shell that had ZLE
} explicitly turned off (as e.g. inside and emacs shell buffer).
} 
} I don't know how that interacts with Peter's patch (I'll try it later when
} I get home).

I should have been able to tell by looking closely at the patch, but this
re-breaks things when interactive in an emacs buffer without doing anything
new and useful for vared when not interactive.

In addition to the bug I previously mentioned:

} If you're going to pull this stuff out into a function, you need three
} return states, not two.

There's also a potential too-early return from bin_vared() a few lines
after the call to openttyspecially(), where `if (zleactive)' is tested,
and several more below that.  Can't just move the place where the tty
gets opened like that.

Peter had asked:

} > I don't think there's any good reason why zle shouldn't be enabled
} > especially.
} 
} In fact, what about this?  If it works for read, shouldn't it work for
} vared?

The difference is that `read' tests specifically for emacs-ness and does
not play with the tty settings in that case.  Running `vared' inside an
emacs buffer is the moral equivalent of running `vi' in one; it simply
can't work at all when it can't drive the editing itself.

You could ALMOST make `vared -c' work, but for any variable that already
has a value emacs is going to treat the old value as part of the prompt.

Given all of this, let's NOT use 11698, and instead use this:

Index: Src/Zle/zle_main.c
===================================================================
@@ -743,7 +743,7 @@
     char *p1 = NULL, *p2 = NULL;
     FILE *oshout = NULL;
 
-    if (unset(USEZLE)) {
+    if ((interact && unset(USEZLE)) || !strcmp(term, "emacs")) {
 	zwarnnam(name, "ZLE not enabled", NULL, 0);
 	return 1;
     }

Even this is not totally satisfactory, because modern versions of emacs
export TERM=dumb and EMACS=t rather than TERM=emacs.  However, TERM=emacs
is the canonical way to tell zsh it's in an emacs buffer, so I've had this
in my .zshrc for a while now:

case ${TERM:-dumb} in
emacs|dumb)
    [[ $EMACS = t ]] || return 0
    TERM=emacs
    export EDITOR=emacs
    stty -onlcr -echo
    ttyctl -f			# Doesn't really do any good
    ;;
eterm)
    TERM=vt100
    ;;
esac

-- 
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: BUG: vared in script (was: vared in function)

[Bernd Eggink]

On Wed, May 31, 2000 at 10:55:21PM +0100, Peter Stephenson wrote:
> Bernd Eggink wrote:
> > Argh... 
> > 
> >    # t
> >    vared -cp "Aha: " BLAH
> > 
> >    % t
> >    t:vared:2: ZLE not enabled
> > 
> > This breaks lots of my scripts! 
> > On the command line, 'vared' works. 
> 
> (I take it that should be `script' rather than `function' in the subject
> line, functions are OK.)

Yes, of course. It has been rather late in the evening...

> I don't think there's any good reason why zle shouldn't be enabled
> especially, if it can find a terminal which it can, but until whoever made
> the change takes a look you can workaround it by giving zsh the -i option
> to run the script interactively.  That might have other side effects,
> though.

It has.

    # t
    vared -cp "Aha: " BLAH
    print $BLAH

    % zsh -i t

    Aha: hm
    hm
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.
    t:zsh:3: use 'exit' to exit.


Regards,
Bernd

-- 
Bernd Eggink
Regionales Rechenzentrum der Uni Hamburg
eggink@uni-hamburg.de
http://www.rrz.uni-hamburg.de/eggink/BEggink.html