patch to make $TERMINFO special

patch to make $TERMINFO special

[Danek Duvall]

I'm sure I'm going about this the wrong way, but at least it seems to work ...

My problem is that I'm using a value of $TERM that's not present in the
system's terminfo database, so when zsh starts up, its terminal handling is
pretty confused.

I'd like to be able to set TERMINFO to the proper path, but in order for
that to do anything, the terminal needs to be reinitialized.  So I've made
TERMINFO a special variable that calls init_term() when it's set, and it
seems to work okay.  I'm guessing that the addenv() call isn't necessary,
but I can't figure out how it ought to work.

    --- zsh-4.3.10/Src/params.c	Fri May  8 02:45:13 2009
    +++ zsh-4.3.10/Src/params.c	Sun May  8 13:09:53 2011
    @@ -198,6 +198,8 @@
     { homegetfn, homesetfn, stdunsetfn };
     static const struct gsu_scalar term_gsu =
     { termgetfn, termsetfn, stdunsetfn };
    +static const struct gsu_scalar terminfo_gsu =
    +{ strgetfn, terminfosetfn, stdunsetfn };
     static const struct gsu_scalar wordchars_gsu =
     { wordcharsgetfn, wordcharssetfn, stdunsetfn };
     static const struct gsu_scalar ifs_gsu =
    @@ -270,6 +272,7 @@
     IPDEF2("histchars", histchars_gsu, PM_DONTIMPORT),
     IPDEF2("HOME", home_gsu, PM_UNSET),
     IPDEF2("TERM", term_gsu, 0),
    +IPDEF2("TERMINFO", terminfo_gsu, 0),
     IPDEF2("WORDCHARS", wordchars_gsu, 0),
     IPDEF2("IFS", ifs_gsu, PM_DONTIMPORT),
     IPDEF2("_", underscore_gsu, PM_READONLY),
    @@ -3943,6 +3946,22 @@
         term = x ? x : ztrdup("");
     
         /* If non-interactive, delay setting up term till we need it. */
    +    if (unset(INTERACTIVE) || !*term)
    +	termflags |= TERM_UNKNOWN;
    +    else 
    +	init_term();
    +}
    +
    +/* Function to set value of special parameter `TERMINFO' */
    +
    +/**/
    +void
    +terminfosetfn(Param pm, char *x)
    +{
    +    strsetfn(pm, x);
    +    addenv(pm, x);
    +
    +    /* If non-interactive, delay setting up term till we need it. */
         if (unset(INTERACTIVE) || !*term)
            termflags |= TERM_UNKNOWN;
         else 

All comments appreciated.

Thanks,
Danek

Re: patch to make $TERMINFO special

[Peter Stephenson]

On Sun, 8 May 2011 13:13:12 -0700
Danek Duvall <duvall@la-z-boy.comfychair.org> wrote:
> I'm sure I'm going about this the wrong way, but at least it seems to
> work ...
> 
> My problem is that I'm using a value of $TERM that's not present in
> the system's terminfo database, so when zsh starts up, its terminal
> handling is pretty confused.
> 
> I'd like to be able to set TERMINFO to the proper path, but in order
> for that to do anything, the terminal needs to be reinitialized.  So
> I've made TERMINFO a special variable that calls init_term() when
> it's set, and it seems to work okay.

I don't see an obvious problem with the basic idea.  What you're
supposed to do is assign a value (including the same value) to TERM
after setting TERMINFO, as documented in the manual, but maybe it could
be more natural.

It means you can't use the variable TERMINFO for other purposes but you
wouldn't want to if you have terminfo.  That's widespread enough I can't
see a good reason for making this dependent on the configuration.

One possible problem is now the TERMINFO variable appears as if it's
always set: you can explicitly unset it but special variables are set by
default.  That's not ideal since you might want to test if it's set to a
useful value.  I think it's safe and sensible to mark it as initially
unset; we do that with "HOME".  This doesn't stop it being imported.

Slightly superstitiously, I'd quite like to keep the TERMINFO code
similar to the TERM code --- though they're not quite the same given
that TERMINFO isn't set by default.

> I'm guessing that the addenv()
> call isn't necessary, but I can't figure out how it ought to work.

What problem is the addenv() call trying to fix?

Index: Doc/Zsh/params.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/params.yo,v
retrieving revision 1.62
diff -p -u -r1.62 params.yo
--- Doc/Zsh/params.yo	17 Dec 2010 17:10:48 -0000	1.62
+++ Doc/Zsh/params.yo	9 May 2011 11:15:37 -0000
@@ -1273,6 +1273,13 @@ is necessary to make such an assignment 
 definition database or terminal type in order for the new settings to
 take effect.
 )
+vindex(TERMINFO)
+item(tt(TERMINFO) <S>)(
+A reference to a compiled description of the terminal, used by the
+`terminfo' library when the system has it; see manref(terminfo)(5).
+If set, this causes the shell to reinitialise the terminal, making
+the workaround `tt(TERM=$TERM)' unnecessary.
+)
 vindex(TIMEFMT)
 item(tt(TIMEFMT))(
 The format of process time reports with the tt(time) keyword.
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.170
diff -p -u -r1.170 params.c
--- Src/params.c	9 May 2011 09:49:09 -0000	1.170
+++ Src/params.c	9 May 2011 11:15:37 -0000
@@ -86,6 +86,7 @@ mod_export
 char *ifs,		/* $IFS         */
      *postedit,		/* $POSTEDIT    */
      *term,		/* $TERM        */
+     *zsh_terminfo,     /* $TERMINFO    */
      *ttystrname,	/* $TTY         */
      *pwd;		/* $PWD         */
 
@@ -202,6 +203,8 @@ static const struct gsu_scalar home_gsu 
 { homegetfn, homesetfn, stdunsetfn };
 static const struct gsu_scalar term_gsu =
 { termgetfn, termsetfn, stdunsetfn };
+static const struct gsu_scalar terminfo_gsu =
+{ terminfogetfn, terminfosetfn, stdunsetfn };
 static const struct gsu_scalar wordchars_gsu =
 { wordcharsgetfn, wordcharssetfn, stdunsetfn };
 static const struct gsu_scalar ifs_gsu =
@@ -276,6 +279,7 @@ IPDEF2("-", dash_gsu, PM_READONLY),
 IPDEF2("histchars", histchars_gsu, PM_DONTIMPORT),
 IPDEF2("HOME", home_gsu, PM_UNSET),
 IPDEF2("TERM", term_gsu, 0),
+IPDEF2("TERMINFO", terminfo_gsu, PM_UNSET),
 IPDEF2("WORDCHARS", wordchars_gsu, 0),
 IPDEF2("IFS", ifs_gsu, PM_DONTIMPORT),
 IPDEF2("_", underscore_gsu, PM_READONLY),
@@ -4045,6 +4049,18 @@ underscoregetfn(UNUSED(Param pm))
     return u;
 }
 
+/* Function used when we need to reinitialies the terminal */
+
+static void
+term_reinit_from_pm(void)
+{
+    /* If non-interactive, delay setting up term till we need it. */
+    if (unset(INTERACTIVE) || !*term)
+	termflags |= TERM_UNKNOWN;
+    else
+	init_term();
+}
+
 /* Function to get value for special parameter `TERM' */
 
 /**/
@@ -4062,12 +4078,27 @@ termsetfn(UNUSED(Param pm), char *x)
 {
     zsfree(term);
     term = x ? x : ztrdup("");
+    term_reinit_from_pm();
+}
 
-    /* If non-interactive, delay setting up term till we need it. */
-    if (unset(INTERACTIVE) || !*term)
-	termflags |= TERM_UNKNOWN;
-    else 
-	init_term();
+/* Function to get value of special parameter `TERMINFO' */
+
+/**/
+char *
+terminfogetfn(Param pm, char *x)
+{
+    return zsh_terminfo ? zsh_terminfo : dupstring("");
+}
+
+/* Function to set value of special parameter `TERMINFO' */
+
+/**/
+void
+terminfosetfn(Param pm, char *x)
+{
+    zsfree(zsh_terminfo);
+    zsh_terminfo = x;
+    term_reinit_from_pm();
 }
 
 /* Function to get value for special parameter `pipestatus' */
-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: patch to make $TERMINFO special

[Danek Duvall]

On Mon, May 09, 2011 at 12:16:53PM +0100, Peter Stephenson wrote:

> I don't see an obvious problem with the basic idea.  What you're
> supposed to do is assign a value (including the same value) to TERM
> after setting TERMINFO, as documented in the manual, but maybe it could
> be more natural.

Argh.  I completely neglected to look for that.

> Slightly superstitiously, I'd quite like to keep the TERMINFO code
> similar to the TERM code

That was my original intent, but it I couldn't get it to work without the
addenv() call, and having the extra internal variable seemed like it was
more code than was really needed.

> > I'm guessing that the addenv() call isn't necessary, but I can't figure
> > out how it ought to work.
> 
> What problem is the addenv() call trying to fix?

It didn't actually seem that TERMINFO was being set in my shell if I didn't
do that.  In fact, I got the message

    can't find terminal definition for xterm-256color

when I change $TERMINFO, and I get that with your patch as well.  It also
just doesn't work -- colors are wrong, tput colors comes back empty, as
does echoti colors, and the terminal handling is absolutely awful, just as
if nothing had happened.

Danek

Re: patch to make $TERMINFO special

[Peter Stephenson]

On Mon, 9 May 2011 08:51:38 -0700
Danek Duvall <duvall@la-z-boy.comfychair.org> wrote:
> > What problem is the addenv() call trying to fix?
> 
> It didn't actually seem that TERMINFO was being set in my shell if I
> didn't do that.  In fact, I got the message
> 
>     can't find terminal definition for xterm-256color
> 
> when I change $TERMINFO, and I get that with your patch as well.  It
> also just doesn't work -- colors are wrong, tput colors comes back
> empty, as does echoti colors, and the terminal handling is absolutely
> awful, just as if nothing had happened.

You are remembering to export it from the shell ("export TERMINFO=...")?
The shell doesn't do that by default and I don't think there's any good
reason to make it do so --- every other variable that needs exporting
you have to mark explicitly for export, unless it was imported.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: patch to make $TERMINFO special

[Danek Duvall]

On Mon, May 09, 2011 at 05:20:14PM +0100, Peter Stephenson wrote:

> On Mon, 9 May 2011 08:51:38 -0700
> Danek Duvall <duvall@la-z-boy.comfychair.org> wrote:
> > > What problem is the addenv() call trying to fix?
> > 
> > It didn't actually seem that TERMINFO was being set in my shell if I
> > didn't do that.  In fact, I got the message
> > 
> >     can't find terminal definition for xterm-256color
> > 
> > when I change $TERMINFO, and I get that with your patch as well.  It
> > also just doesn't work -- colors are wrong, tput colors comes back
> > empty, as does echoti colors, and the terminal handling is absolutely
> > awful, just as if nothing had happened.
> 
> You are remembering to export it from the shell ("export TERMINFO=...")?

Yes:

    export TERMINFO=/var/tmp/dduvall/.terminfo

Danek

Re: patch to make $TERMINFO special

[Peter Stephenson]

On Mon, 9 May 2011 09:23:02 -0700
Danek Duvall <duvall@la-z-boy.comfychair.org> wrote:
> On Mon, May 09, 2011 at 05:20:14PM +0100, Peter Stephenson wrote:
> 
> > On Mon, 9 May 2011 08:51:38 -0700
> > Danek Duvall <duvall@la-z-boy.comfychair.org> wrote:
> > > > What problem is the addenv() call trying to fix?
> > > 
> > > It didn't actually seem that TERMINFO was being set in my shell
> > > if I didn't do that.  In fact, I got the message
> > > 
> > >     can't find terminal definition for xterm-256color
> > > 
> > > when I change $TERMINFO, and I get that with your patch as well.
> > > It also just doesn't work -- colors are wrong, tput colors comes
> > > back empty, as does echoti colors, and the terminal handling is
> > > absolutely awful, just as if nothing had happened.
> > 
> > You are remembering to export it from the shell ("export
> > TERMINFO=...")?
> 
> Yes:
> 
>     export TERMINFO=/var/tmp/dduvall/.terminfo

Hmmm... the code that sets the exported value (that terminfo uses) is
probably being set later, in which we do need to fix up exports.  I
think it should be conditionalised, however...

Index: Doc/Zsh/params.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/params.yo,v
retrieving revision 1.62
diff -p -u -r1.62 params.yo
--- Doc/Zsh/params.yo	17 Dec 2010 17:10:48 -0000	1.62
+++ Doc/Zsh/params.yo	9 May 2011 17:01:21 -0000
@@ -1273,6 +1273,13 @@ is necessary to make such an assignment 
 definition database or terminal type in order for the new settings to
 take effect.
 )
+vindex(TERMINFO)
+item(tt(TERMINFO) <S>)(
+A reference to a compiled description of the terminal, used by the
+`terminfo' library when the system has it; see manref(terminfo)(5).
+If set, this causes the shell to reinitialise the terminal, making
+the workaround `tt(TERM=$TERM)' unnecessary.
+)
 vindex(TIMEFMT)
 item(tt(TIMEFMT))(
 The format of process time reports with the tt(time) keyword.
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.170
diff -p -u -r1.170 params.c
--- Src/params.c	9 May 2011 09:49:09 -0000	1.170
+++ Src/params.c	9 May 2011 17:01:21 -0000
@@ -86,6 +86,7 @@ mod_export
 char *ifs,		/* $IFS         */
      *postedit,		/* $POSTEDIT    */
      *term,		/* $TERM        */
+     *zsh_terminfo,     /* $TERMINFO    */
      *ttystrname,	/* $TTY         */
      *pwd;		/* $PWD         */
 
@@ -202,6 +203,8 @@ static const struct gsu_scalar home_gsu 
 { homegetfn, homesetfn, stdunsetfn };
 static const struct gsu_scalar term_gsu =
 { termgetfn, termsetfn, stdunsetfn };
+static const struct gsu_scalar terminfo_gsu =
+{ terminfogetfn, terminfosetfn, stdunsetfn };
 static const struct gsu_scalar wordchars_gsu =
 { wordcharsgetfn, wordcharssetfn, stdunsetfn };
 static const struct gsu_scalar ifs_gsu =
@@ -276,6 +279,7 @@ IPDEF2("-", dash_gsu, PM_READONLY),
 IPDEF2("histchars", histchars_gsu, PM_DONTIMPORT),
 IPDEF2("HOME", home_gsu, PM_UNSET),
 IPDEF2("TERM", term_gsu, 0),
+IPDEF2("TERMINFO", terminfo_gsu, PM_UNSET),
 IPDEF2("WORDCHARS", wordchars_gsu, 0),
 IPDEF2("IFS", ifs_gsu, PM_DONTIMPORT),
 IPDEF2("_", underscore_gsu, PM_READONLY),
@@ -4045,6 +4049,18 @@ underscoregetfn(UNUSED(Param pm))
     return u;
 }
 
+/* Function used when we need to reinitialies the terminal */
+
+static void
+term_reinit_from_pm(void)
+{
+    /* If non-interactive, delay setting up term till we need it. */
+    if (unset(INTERACTIVE) || !*term)
+	termflags |= TERM_UNKNOWN;
+    else
+	init_term();
+}
+
 /* Function to get value for special parameter `TERM' */
 
 /**/
@@ -4062,12 +4078,35 @@ termsetfn(UNUSED(Param pm), char *x)
 {
     zsfree(term);
     term = x ? x : ztrdup("");
+    term_reinit_from_pm();
+}
 
-    /* If non-interactive, delay setting up term till we need it. */
-    if (unset(INTERACTIVE) || !*term)
-	termflags |= TERM_UNKNOWN;
-    else 
-	init_term();
+/* Function to get value of special parameter `TERMINFO' */
+
+/**/
+char *
+terminfogetfn(Param pm, char *x)
+{
+    return zsh_terminfo ? zsh_terminfo : dupstring("");
+}
+
+/* Function to set value of special parameter `TERMINFO' */
+
+/**/
+void
+terminfosetfn(Param pm, char *x)
+{
+    zsfree(zsh_terminfo);
+    zsh_terminfo = x;
+
+    /*
+     * terminfo relies on the value being exported before
+     * we reinitialise the terminal.  This is a bit inefficient.
+     */
+    if ((pm->node.flags & PM_EXPORTED) && x)
+	addenv(pm, x);
+
+    term_reinit_from_pm();
 }
 
 /* Function to get value for special parameter `pipestatus' */

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: patch to make $TERMINFO special

[Danek Duvall]

On Mon, May 09, 2011 at 06:02:26PM +0100, Peter Stephenson wrote:

> Hmmm... the code that sets the exported value (that terminfo uses) is
> probably being set later, in which we do need to fix up exports.  I
> think it should be conditionalised, however...

Okay.  FWIW, I'm happy with the TERM=$TERM solution, too.

> +/* Function used when we need to reinitialies the terminal */

"reinitialise"

> +/**/
> +char *
> +terminfogetfn(Param pm, char *x)

The second parameter causes the compiler to emit a warning, and the first
should probably be marked UNUSED().

Danek

Re: patch to make $TERMINFO special

[Peter Stephenson]

On Mon, 9 May 2011 10:16:21 -0700
Danek Duvall <duvall@la-z-boy.comfychair.org> wrote:
> On Mon, May 09, 2011 at 06:02:26PM +0100, Peter Stephenson wrote:
> 
> > Hmmm... the code that sets the exported value (that terminfo uses)
> > is probably being set later, in which we do need to fix up
> > exports.  I think it should be conditionalised, however...
> 
> Okay.  FWIW, I'm happy with the TERM=$TERM solution, too.

It's probably worth the effort to make TERMINFO special to avoid needing
people to rummage in the basement.  As I said, TERMINFO is a specialised
enough name it's hard to see this causing a problem.
 
> > +/* Function used when we need to reinitialies the terminal */
> 
> "reinitialise"
> 
> > +/**/
> > +char *
> > +terminfogetfn(Param pm, char *x)
> 
> The second parameter causes the compiler to emit a warning, and the
> first should probably be marked UNUSED().

I've fixed those and commited it.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom