PATCH: curses tweaks, maybe

PATCH: curses tweaks, maybe

[Peter Stephenson]

I had a short play with zcurses.  Well, it started off as a short play.
By the way, I have
ncurses-devel-5.6-6.20070303.fc7
ncurses-5.6-6.20070303.fc7

We need to free up the linked list data element after use, on zcurses -d.

Also, a refresh() is needed after deleting a window, otherwise nothing
works when you add a new one.  It's not clear where this should be,
however.  If the intent is that the user can stack up commands before
refreshing we would need that as a user command, but we might well
need to ensure that it got called anyway before things got too hopefully
messed up (e.g. it would probably need to be called just before, or
presumably in place of, a wrefresh() for a new window).  So this will do
for now.


Also, something very screwy was happening with stty onlcr on my terminal,
which keeps being turned off when running the updated test function,

curses_test() {
  zmodload zsh/curses
  zcurses -a $win 10 10 10 10
  zcurses -b $win
  zcurses -m $win 1 1
  zcurses -c $win B
  zcurses -c $win l
  zcurses -c $win a
  zcurses -c $win h
  zcurses -r $win
  sleep 5
  zcurses -d $win
}

I think I've tracked it down to this: if zsh/curses gets loaded here,
onclr is turned off; if zsh/curses was already loaded then it's OK.
Presumably the difference is that from the command line zsh is doing it's
sanity checks (but I don't quite understand why they don't happen after the
above).  This is from initscr().  The functions nonl() / nl() seem to
control this in curses, but they docs suggest the setting isn't changed
automatically, which seems to be wrong.

Perhaps to be safe we should be requiring all use of curses to be between
commands such as zcurses -i and zcurses -e, as below?  After all, it's
reasonable to suppose this doesn't fit in with normal shell line-by-line
character handling.  This fixes Bart's gripe about clearing the screen.

Even with this code I only got it to work by both saving the terminal state
from before zcurses -i, and when I restore it on zcurses -e ensuring that
shttyinfo is the same---this seems a bit hairy.  (I had a panic attack that
shttyinfo.winsize would be screwed up, too, but
settyinfo() doesn't alter that.)

The test now becomes:

curses_test() {
  zmodload zsh/curses
  zcurses -i
  zcurses -a $win 10 10 10 10
  zcurses -b $win
  zcurses -m $win 1 1
  zcurses -c $win B
  zcurses -c $win l
  zcurses -c $win a
  zcurses -c $win h
  zcurses -r $win
  sleep 5
  zcurses -d $win
  zcurses -e
}

This seemed to work.   Probably zcurses -e, if it stays, should do more
work, such as deleting any remaining windows.  (That could fix up the
refresh() issue mentioned at the top in the case where we delete all
windows.)

zcurses -i could be implicit, but it's nice to be clear.  Maybe it should
return an error if win_zero is already set, or allow nested -i/-e pairs, or
something.

I'm not sure how convinced I am by all of this.

Index: Doc/Zsh/mod_curses.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_curses.yo,v
retrieving revision 1.4
diff -u -r1.4 mod_curses.yo
--- Doc/Zsh/mod_curses.yo	15 Oct 2007 16:49:50 -0000	1.4
+++ Doc/Zsh/mod_curses.yo	15 Oct 2007 18:28:15 -0000
@@ -6,6 +6,8 @@
 startitem()
 findex(zcurses)
 cindex(windows, curses)
+xitem(tt(zcurses) tt(-i))
+xitem(tt(zcurses) tt(-e))
 xitem(tt(zcurses) tt(-a) var(targetwin) var(nlines) var(ncols) var(begin_y) var(begin_x) )
 xitem(tt(zcurses) tt(-d) var(targetwin) )
 xitem(tt(zcurses) tt(-r) var(targetwin) )
@@ -13,7 +15,10 @@
 xitem(tt(zcurses) tt(-c) var(targetwin) var(character) )
 xitem(tt(zcurses) tt(-s) var(targetwin) var(string) )
 item(tt(zcurses) tt(-b) var(targetwin) var(border) )(
-Manipulate curses windows.
+Manipulate curses windows.  All uses of this command should be
+bracketed by `tt(zcurses -i)' to initialise use of curses, and
+`tt(zcurses -e)' to end it; omitting `tt(zcurses -e)' can cause
+the terminal to be in an unwanted state.
 
 With tt(-a), create a window with var(nlines) lines and var(ncols) columns.
 Its upper left corner will be placed at row var(begin_y) and column
Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.8
diff -u -r1.8 curses.c
--- Src/Modules/curses.c	15 Oct 2007 16:57:48 -0000	1.8
+++ Src/Modules/curses.c	15 Oct 2007 18:28:15 -0000
@@ -44,8 +44,9 @@
     char *name;
 } *ZCWin;
 
-WINDOW *win_zero;
-LinkList zcurses_windows;
+static WINDOW *win_zero;
+static struct ttyinfo saved_tty_state;
+static LinkList zcurses_windows;
 
 #define ZCURSES_ERANGE 1
 #define ZCURSES_EDEFINED 2
@@ -125,6 +126,15 @@
 static int
 bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
 {
+    /* Initialise curses */
+    if (OPT_ISSET(ops,'i')) {
+	if (!win_zero) {
+	    gettyinfo(&saved_tty_state);
+	    win_zero = initscr();
+	}
+	return 0;
+    }
+
     if (OPT_ISSET(ops,'a')) {
 	int nlines, ncols, begin_y, begin_x;
         ZCWin w;
@@ -179,7 +189,8 @@
 	if (w->name)
 	    zsfree(w->name);
 
-        remnode(zcurses_windows, node);
+        zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));
+	refresh();
 
 	return 0;
     }
@@ -325,6 +336,24 @@
 	return 0;
     }
 
+    /* Finish using curses */
+    if (OPT_ISSET(ops,'e')) {
+	if (win_zero) {
+	    endwin();
+	    /* TODO: do we free win_zero?  Manual doesn't say. */
+	    win_zero = NULL;
+	    /* Restore TTY as it was before zcurses -i */
+	    settyinfo(&saved_tty_state);
+	    /*
+	     * TODO: should I need the following?  Without it
+	     * the screen stays messed up.  Presumably we are
+	     * doing stuff with shttyinfo when we shouldn't really be.
+	     */
+	    gettyinfo(&shttyinfo);
+	}
+	return 0;
+    }
+
     return 0;
 }
 
@@ -333,7 +362,7 @@
  */
 
 static struct builtin bintab[] = {
-    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "ab:cd:mr:rs", NULL),
+    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "ab:cd:eimr:rs", NULL),
 };
 
 static struct features module_features = {
@@ -371,7 +400,6 @@
 boot_(Module m)
 {
     zcurses_windows = znewlinklist();
-    win_zero=initscr();
 
     return 0;
 }
@@ -380,7 +408,6 @@
 int
 cleanup_(Module m)
 {
-    endwin();
     freelinklist(zcurses_windows, (FreeFunc) zcurses_free_window);
     return setfeatureenables(m, &module_features, NULL);
 }


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

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Tue, Oct 16, 2007 at 09:40:40AM +0100, Peter Stephenson wrote:
> Also, a refresh() is needed after deleting a window, otherwise nothing
> works when you add a new one.  It's not clear where this should be,
> however.  If the intent is that the user can stack up commands before
> refreshing we would need that as a user command, but we might well
> need to ensure that it got called anyway before things got too hopefully
> messed up (e.g. it would probably need to be called just before, or
> presumably in place of, a wrefresh() for a new window).  So this will do
> for now.

I think we should provide refresh() as zcurses -R.  Apparently (at least
according to http://invisible-island.net/ncurses/ncurses-intro.html ;
I've not tried it) we can do an endwin() with zcurses -e, mess around with
normal shell output, then redraw with refresh() and resume where we've
left off.

> I think I've tracked it down to this: if zsh/curses gets loaded here,
> onclr is turned off; if zsh/curses was already loaded then it's OK.
> Presumably the difference is that from the command line zsh is doing it's
> sanity checks (but I don't quite understand why they don't happen after the
> above).  This is from initscr().  The functions nonl() / nl() seem to
> control this in curses, but they docs suggest the setting isn't changed
> automatically, which seems to be wrong.

I think I just read somewhere (though I can't find it now) that nl() is
the default and that nonl() provides speed benefits if you don't need
the translation.  Maybe we should unconditionally run nonl() after
initscr(), since we don't support (yet?) any curses input functionality
and anyone passing a literal newline to zcurses -s (does this work for
you either?) can probably be bothered to pass a CR too.

> Perhaps to be safe we should be requiring all use of curses to be between
> commands such as zcurses -i and zcurses -e, as below?  After all, it's
> reasonable to suppose this doesn't fit in with normal shell line-by-line
> character handling.  This fixes Bart's gripe about clearing the screen.

We could; maybe they could take arguments and initialize/delete
multiple terminals/screens if we want to support that as well (and skip
initscr() altogether).

> Even with this code I only got it to work by both saving the terminal state
> from before zcurses -i, and when I restore it on zcurses -e ensuring that
> shttyinfo is the same---this seems a bit hairy.  (I had a panic attack that
> shttyinfo.winsize would be screwed up, too, but
> settyinfo() doesn't alter that.)

I wonder why this is necessary.

> This seemed to work.   Probably zcurses -e, if it stays, should do more
> work, such as deleting any remaining windows.  (That could fix up the
> refresh() issue mentioned at the top in the case where we delete all
> windows.)

Maybe zcurses -d with no arguments could delete all, or we could have a
separate endwin()-only command for use in the endwin()/refresh() trick
described up top.

Re: PATCH: curses tweaks, maybe

[Peter Stephenson]

On Tue, 16 Oct 2007 23:29:55 -0400
Clint Adams <clint@zsh.org> wrote:
> I think we should provide refresh() as zcurses -R.

That's easy.

> Apparently (at least
> according to http://invisible-island.net/ncurses/ncurses-intro.html ;
> I've not tried it) we can do an endwin() with zcurses -e, mess around with
> normal shell output, then redraw with refresh() and resume where we've
> left off.

Presumably that means we still need -i and -e to swap the terminal
settings, but it should save the values from curses mode, too, as below.
However, this didn't work for me:  the window didn't get redrawn.  Do we
need initscr() again, or something else?

The previous test script, which still works, is now

  zmodload zsh/curses

  integer win=6

  zcurses -i

  zcurses -a $win 10 10 10 10
  zcurses -b $win
  zcurses -m $win 1 1
  zcurses -c $win B
  zcurses -c $win l
  zcurses -c $win a
  zcurses -c $win h
  zcurses -r $win
  sleep 5
  zcurses -d $win
  zcurses -R

  zcurses -e

> I think I just read somewhere (though I can't find it now) that nl() is
> the default and that nonl() provides speed benefits if you don't need
> the translation.  Maybe we should unconditionally run nonl() after
> initscr(), since we don't support (yet?) any curses input functionality
> and anyone passing a literal newline to zcurses -s (does this work for
> you either?) can probably be bothered to pass a CR too.

That's probably fine, but I don't suppose it matters all that much.  I
haven't got around to zcurses -s yet.

I haven't changed anything else from the previous version of the patch, but
obviously many further enhancements are possible.

Index: Doc/Zsh/mod_curses.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_curses.yo,v
retrieving revision 1.4
diff -u -r1.4 mod_curses.yo
--- Doc/Zsh/mod_curses.yo	15 Oct 2007 16:49:50 -0000	1.4
+++ Doc/Zsh/mod_curses.yo	17 Oct 2007 08:37:02 -0000
@@ -6,6 +6,8 @@
 startitem()
 findex(zcurses)
 cindex(windows, curses)
+xitem(tt(zcurses) tt(-i))
+xitem(tt(zcurses) tt(-e))
 xitem(tt(zcurses) tt(-a) var(targetwin) var(nlines) var(ncols) var(begin_y) var(begin_x) )
 xitem(tt(zcurses) tt(-d) var(targetwin) )
 xitem(tt(zcurses) tt(-r) var(targetwin) )
@@ -13,7 +15,10 @@
 xitem(tt(zcurses) tt(-c) var(targetwin) var(character) )
 xitem(tt(zcurses) tt(-s) var(targetwin) var(string) )
 item(tt(zcurses) tt(-b) var(targetwin) var(border) )(
-Manipulate curses windows.
+Manipulate curses windows.  All uses of this command should be
+bracketed by `tt(zcurses -i)' to initialise use of curses, and
+`tt(zcurses -e)' to end it; omitting `tt(zcurses -e)' can cause
+the terminal to be in an unwanted state.

 With tt(-a), create a window with var(nlines) lines and var(ncols) columns.
 Its upper left corner will be placed at row var(begin_y) and column
Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.8
diff -u -r1.8 curses.c
--- Src/Modules/curses.c	15 Oct 2007 16:57:48 -0000	1.8
+++ Src/Modules/curses.c	17 Oct 2007 08:37:02 -0000
@@ -44,8 +44,10 @@
     char *name;
 } *ZCWin;

-WINDOW *win_zero;
-LinkList zcurses_windows;
+static WINDOW *win_zero;
+static struct ttyinfo saved_tty_state;
+static struct ttyinfo curses_tty_state;
+static LinkList zcurses_windows;

 #define ZCURSES_ERANGE 1
 #define ZCURSES_EDEFINED 2
@@ -125,6 +127,18 @@
 static int
 bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
 {
+    /* Initialise curses */
+    if (OPT_ISSET(ops,'i')) {
+	if (!win_zero) {
+	    gettyinfo(&saved_tty_state);
+	    win_zero = initscr();
+	    gettyinfo(&curses_tty_state);
+	} else {
+	    settyinfo(&curses_tty_state);
+	}
+	return 0;
+    }
+
     if (OPT_ISSET(ops,'a')) {
 	int nlines, ncols, begin_y, begin_x;
         ZCWin w;
@@ -179,7 +193,7 @@
 	if (w->name)
 	    zsfree(w->name);

-        remnode(zcurses_windows, node);
+        zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));

 	return 0;
     }
@@ -199,6 +213,11 @@
 	return (wrefresh(w->win)!=OK) ? 1 : 0;
     }

+    if (OPT_ISSET(ops,'R')) {
+	refresh();
+	return 0;
+    }
+
     if (OPT_ISSET(ops,'m')) {
 	int y, x;
 	LinkNode node;
@@ -325,6 +344,22 @@
 	return 0;
     }

+    /* Finish using curses */
+    if (OPT_ISSET(ops,'e')) {
+	if (win_zero) {
+	    endwin();
+	    /* Restore TTY as it was before zcurses -i */
+	    settyinfo(&saved_tty_state);
+	    /*
+	     * TODO: should I need the following?  Without it
+	     * the screen stays messed up.  Presumably we are
+	     * doing stuff with shttyinfo when we shouldn't really be.
+	     */
+	    gettyinfo(&shttyinfo);
+	}
+	return 0;
+    }
+
     return 0;
 }

@@ -333,7 +368,7 @@
  */

 static struct builtin bintab[] = {
-    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "ab:cd:mr:rs", NULL),
+    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "ab:cd:eimr:Rs", NULL),
 };

 static struct features module_features = {
@@ -371,7 +406,6 @@
 boot_(Module m)
 {
     zcurses_windows = znewlinklist();
-    win_zero=initscr();

     return 0;
 }
@@ -380,7 +414,6 @@
 int
 cleanup_(Module m)
 {
-    endwin();
     freelinklist(zcurses_windows, (FreeFunc) zcurses_free_window);
     return setfeatureenables(m, &module_features, NULL);
 }


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

Re: PATCH: curses tweaks, maybe

[Peter Stephenson]

Peter Stephenson wrote:
> On Tue, 16 Oct 2007 23:29:55 -0400
> Clint Adams <clint@zsh.org> wrote:
> > I think we should provide refresh() as zcurses -R.
> 
> That's easy.

Actually, wouldn't it be simpler to use "zcurses -r" with no argument
for this?  (Also I forgot to document the change.)

If I commit anything I'll incorporate Vin's patch.

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

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Wed, Oct 17, 2007 at 10:14:44AM +0100, Peter Stephenson wrote:
> Actually, wouldn't it be simpler to use "zcurses -r" with no argument
> for this?  (Also I forgot to document the change.)

Yes, I imagine so.  We could also special-case the window
name "stdscr".

Re: PATCH: curses tweaks, maybe

[Bart Schaefer]

On Oct 17, 10:14am, Peter Stephenson wrote:
}
} Peter Stephenson wrote:
} > On Tue, 16 Oct 2007 23:29:55 -0400
} > Clint Adams <clint@zsh.org> wrote:
} > > I think we should provide refresh() as zcurses -R.
} > 
} > That's easy.
} 
} Actually, wouldn't it be simpler to use "zcurses -r" with no argument
} for this?  (Also I forgot to document the change.)

Maybe lower-case options should require a window name and upper case
should require that there NOT be a window name?

Then there'd be -R -I and -E for refresh/initscr/endwin.

I don't feel strongly, just suggesting.

Re: PATCH: curses tweaks, maybe

[Peter Stephenson]

Bart Schaefer wrote:
> Maybe lower-case options should require a window name and upper case
> should require that there NOT be a window name?
> 
> Then there'd be -R -I and -E for refresh/initscr/endwin.

I had this horrible feeling we'd be running out of letters if we tried
to do anything too clever too early, but it's really just raw paranoia.

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

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Wed, Oct 17, 2007 at 04:05:44PM +0100, Peter Stephenson wrote:
> I had this horrible feeling we'd be running out of letters if we tried
> to do anything too clever too early, but it's really just raw paranoia.

Well, there's plenty of the curses API that we haven't touched yet.
Next I'll be needing attributes and colors; does anyone have UI
preferences for wattrset() and wcolor_set()?

Re: PATCH: curses tweaks, maybe

[Peter Stephenson]

Clint Adams wrote:
> Next I'll be needing attributes and colors; does anyone have UI
> preferences for wattrset() and wcolor_set()?

Probably something like

zcurses -A window +standout

with +/- to enable and disable options which correspond to attributes
that we can either parse out of the header or hardcode as { A_STANDOUT,
"standout"} etc.

zcurses -C window black/red

(for black on red, obviously---I prefer it with the / since unlike with
a space it's obvious what the foreground is) to which similar arguments
apply.

We need to think about any modifications we might want to "read" to make
this more useful.

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

Re: PATCH: curses tweaks, maybe

[Bart Schaefer]

On Oct 17, 11:25am, Clint Adams wrote:
} Subject: Re: PATCH: curses tweaks, maybe
}
} On Wed, Oct 17, 2007 at 04:05:44PM +0100, Peter Stephenson wrote:
} > I had this horrible feeling we'd be running out of letters if we tried
} > to do anything too clever too early, but it's really just raw paranoia.
} 
} Well, there's plenty of the curses API that we haven't touched yet.
} Next I'll be needing attributes and colors; does anyone have UI
} preferences for wattrset() and wcolor_set()?

In that case maybe option letters are completely out and we ought to be
doing

	zcurses addwin targetwinname ...
	zcurses refresh
	zcurses endwin

etc.

Re: PATCH: curses tweaks, maybe

[Peter Stephenson]

Bart Schaefer wrote:
> In that case maybe option letters are completely out and we ought to be
> doing
> 
> 	zcurses addwin targetwinname ...
> 	zcurses refresh
> 	zcurses endwin

That does make a lot of sense, though we wouldn't want to adopt curses
naming throughout, since that's horrific, as I've just been finding out.

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

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Wed, Oct 17, 2007 at 04:39:19PM +0100, Peter Stephenson wrote:
> zcurses -A window +standout
> 
> with +/- to enable and disable options which correspond to attributes
> that we can either parse out of the header or hardcode as { A_STANDOUT,
> "standout"} etc.

Here goes, and here's my current test script:

-8<-
zcurses -i
zcurses -a tw $(( LINES - 10 )) $(( COLUMNS - 20 )) 5 10
zcurses -b tw
zcurses -m tw 1 1
zcurses -c tw B
zcurses -c tw l
zcurses -c tw a
zcurses -c tw h
zcurses -r tw
zcurses -m tw 2 2
zcurses -s tw String
zcurses -m tw 3 3
zcurses -A tw +bold +underline
zcurses -s tw BoLD
zcurses -r tw
sleep 5
zcurses -e
-8<-

Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.9
diff -u -r1.9 curses.c
--- Src/Modules/curses.c	17 Oct 2007 13:11:11 -0000	1.9
+++ Src/Modules/curses.c	17 Oct 2007 18:56:28 -0000
@@ -44,6 +44,11 @@
     char *name;
 } *ZCWin;
 
+struct zcurses_attribute {
+    char *name;
+    int number;
+};
+
 static WINDOW *win_zero;
 static struct ttyinfo saved_tty_state;
 static struct ttyinfo curses_tty_state;
@@ -56,6 +61,9 @@
 #define ZCURSES_UNUSED 1
 #define ZCURSES_USED 2
 
+#define ZCURSES_ATTRON 1
+#define ZCURSES_ATTROFF 2
+
 static int zc_errno;
 
 static const char *
@@ -123,6 +131,41 @@
     return 0;
 }
 
+static int
+zcurses_attribute(WINDOW *w, char *attr, int op)
+{
+    struct zcurses_attribute *zca;
+
+    static const struct zcurses_attribute zcurses_attributes[] = {
+	{"blink", A_BLINK},
+	{"bold", A_BOLD},
+	{"dim", A_DIM},
+	{"reverse", A_REVERSE},
+	{"standout", A_STANDOUT},
+	{"underline", A_UNDERLINE},
+	{NULL, 0}
+    };
+
+    if (!attr)
+	return 1;
+
+    for(zca=(struct zcurses_attribute *)zcurses_attributes;zca->name;zca++)
+	if (!strcmp(attr, zca->name)) {
+	    switch(op) {
+		case ZCURSES_ATTRON:
+		    wattron(w, zca->number);
+		    break;
+		case ZCURSES_ATTROFF:
+		    wattroff(w, zca->number);
+		    break;
+	    }
+
+	    return 0;
+	}
+
+    return 1;
+}
+
 /**/
 static int
 bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
@@ -141,7 +184,7 @@
 
     if (OPT_ISSET(ops,'a')) {
 	int nlines, ncols, begin_y, begin_x;
-        ZCWin w;
+	ZCWin w;
 
 	if (zcurses_validate_window(args[0], ZCURSES_UNUSED) == NULL && zc_errno) {
 	    zerrnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
@@ -153,11 +196,11 @@
 	begin_y = atoi(args[3]);
 	begin_x = atoi(args[4]);
 
-        w = (ZCWin)zshcalloc(sizeof(struct zc_win));
-        if (!w)
+	w = (ZCWin)zshcalloc(sizeof(struct zc_win));
+	if (!w)
 	    return 1;
 
-        w->name = ztrdup(args[0]);
+	w->name = ztrdup(args[0]);
 	w->win = newwin(nlines, ncols, begin_y, begin_x);
 
 	if (w->win == NULL) {
@@ -166,7 +209,7 @@
 	    return 1;
 	}
 
-        zinsertlinknode(zcurses_windows, lastnode(zcurses_windows), (void *)w);
+	zinsertlinknode(zcurses_windows, lastnode(zcurses_windows), (void *)w);
 
 	return 0;
     }
@@ -193,7 +236,7 @@
 	if (w->name)
 	    zsfree(w->name);
 
-        zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));
+	zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));
 
 	return 0;
     }
@@ -206,7 +249,7 @@
 	    node = zcurses_validate_window(args[0], ZCURSES_USED);
 	    if (node == NULL) {
 		zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0],
-			 0);
+			0);
 		return 1;
 	    }
 
@@ -361,6 +404,37 @@
 	}
 	return 0;
     }
+    if (OPT_ISSET(ops,'A')) {
+	LinkNode node;
+	ZCWin w;
+	char **attrs;
+
+	if (!args[0])
+	    return 1;
+
+	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	if (node == NULL) {
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	    return 1;
+	}
+
+	w = (ZCWin)getdata(node);
+
+	for(attrs = args+1; *attrs; attrs++) {
+	    switch(*attrs[0]) {
+		case '-':
+		    zcurses_attribute(w->win, (*attrs)+1, ZCURSES_ATTROFF);
+		    break;
+		case '+':
+		    zcurses_attribute(w->win, (*attrs)+1, ZCURSES_ATTRON);
+		    break;
+		default:
+		    /* maybe a bad idea to spew warnings here */
+		    break;
+	    }
+	}
+	return 0;
+    }
 
     return 0;
 }
@@ -370,7 +444,7 @@
  */
 
 static struct builtin bintab[] = {
-    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "ab:cd:eimrs", NULL),
+    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "Aab:cd:eimrs", NULL),
 };
 
 static struct features module_features = {

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Wed, Oct 17, 2007 at 02:58:27PM -0400, Clint Adams wrote:
> zcurses -A tw +bold +underline

Some possibly-unclear docs to match.

Index: Doc/Zsh/mod_curses.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_curses.yo,v
retrieving revision 1.5
diff -u -r1.5 mod_curses.yo
--- Doc/Zsh/mod_curses.yo	17 Oct 2007 13:11:11 -0000	1.5
+++ Doc/Zsh/mod_curses.yo	17 Oct 2007 19:17:43 -0000
@@ -14,7 +14,8 @@
 xitem(tt(zcurses) tt(-m) var(targetwin) var(new_y) var(new_x) )
 xitem(tt(zcurses) tt(-c) var(targetwin) var(character) )
 xitem(tt(zcurses) tt(-s) var(targetwin) var(string) )
-item(tt(zcurses) tt(-b) var(targetwin) var(border) )(
+xitem(tt(zcurses) tt(-b) var(targetwin) var(border) )(
+item(tt(zcurses) tt(-A) var(targetwin) var({+/-}attribute) [var({+/-}attribute)] [...])(
 Manipulate curses windows.  All uses of this command should be
 bracketed by `tt(zcurses -i)' to initialise use of curses, and
 `tt(zcurses -e)' to end it; omitting `tt(zcurses -e)' can cause
@@ -39,5 +40,11 @@
 respectively.
 
 To draw a border around window var(targetwin), use tt(-b).
+
+tt(-A) will set var(targetwin)'s attributes for any successive character
+output.  Each var(attribute) given on the line should be prepended by a
+tt(+) to set or a tt(-) to unset that attribute.  The attributes supported
+are tt(blink), tt(bold), tt(dim), tt(reverse), tt(standout), and
+tt(underline).
 )
 enditem()

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Wed, Oct 17, 2007 at 04:39:19PM +0100, Peter Stephenson wrote:
> zcurses -C window black/red

I don't know why this doesn't work; perhaps another man page I'm
misreading.

Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.11
diff -u -r1.11 curses.c
--- Src/Modules/curses.c	18 Oct 2007 08:29:34 -0000	1.11
+++ Src/Modules/curses.c	18 Oct 2007 20:16:02 -0000
@@ -49,7 +49,7 @@
     char *name;
 } *ZCWin;
 
-struct zcurses_attribute {
+struct zcurses_namenumberpair {
     char *name;
     int number;
 };
@@ -58,6 +58,7 @@
 static struct ttyinfo saved_tty_state;
 static struct ttyinfo curses_tty_state;
 static LinkList zcurses_windows;
+static HashTable zcurses_colorpairs;
 
 #define ZCURSES_ERANGE 1
 #define ZCURSES_EDEFINED 2
@@ -69,7 +70,7 @@
 #define ZCURSES_ATTRON 1
 #define ZCURSES_ATTROFF 2
 
-static int zc_errno;
+static int zc_errno, zc_color_phase=0, next_cp=0;
 
 static const char *
 zcurses_strerror(int err)
@@ -139,9 +140,9 @@
 static int
 zcurses_attribute(WINDOW *w, char *attr, int op)
 {
-    struct zcurses_attribute *zca;
+    struct zcurses_namenumberpair *zca;
 
-    static const struct zcurses_attribute zcurses_attributes[] = {
+    static const struct zcurses_namenumberpair zcurses_attributes[] = {
 	{"blink", A_BLINK},
 	{"bold", A_BOLD},
 	{"dim", A_DIM},
@@ -154,7 +155,7 @@
     if (!attr)
 	return 1;
 
-    for(zca=(struct zcurses_attribute *)zcurses_attributes;zca->name;zca++)
+    for(zca=(struct zcurses_namenumberpair *)zcurses_attributes;zca->name;zca++)
 	if (!strcmp(attr, zca->name)) {
 	    switch(op) {
 		case ZCURSES_ATTRON:
@@ -171,6 +172,85 @@
     return 1;
 }
 
+static int
+zcurses_color(char *color)
+{
+    struct zcurses_namenumberpair *zc;
+
+    static const struct zcurses_namenumberpair zcurses_colors[] = {
+	{"black", COLOR_BLACK},
+	{"red", COLOR_RED},
+	{"green", COLOR_GREEN},
+	{"yellow", COLOR_YELLOW},
+	{"blue", COLOR_BLUE},
+	{"magenta", COLOR_MAGENTA},
+	{"cyan", COLOR_CYAN},
+	{"white", COLOR_WHITE},
+	{NULL, 0}
+    };
+
+    for(zc=(struct zcurses_namenumberpair *)zcurses_colors;zc->name;zc++)
+	if (!strcmp(color, zc->name)) {
+	    return zc->number;
+	}
+
+    return -1;
+}
+
+static int
+zcurses_colorset(WINDOW *w, char *colorpair)
+{
+    char *fg, *bg, *cp;
+    int *c, f, b;
+
+    if (zc_color_phase==1 || !(c = (int *) gethashnode(zcurses_colorpairs, colorpair))) {
+	zc_color_phase = 2;
+	cp = ztrdup(colorpair);
+	fg = strtok(cp, "/");
+	bg = strtok(NULL, "/");
+
+	if (bg==NULL) {
+	    zsfree(cp);
+	    return 1;
+	}
+        
+	f = zcurses_color(fg);
+	b = zcurses_color(bg);
+
+	zsfree(cp);
+
+	if (f==-1 || b==-1)
+	    return 1;
+
+	++next_cp;
+	if (next_cp >= COLOR_PAIRS)
+	    return 1;
+
+	if (init_pair(next_cp, f, b) == ERR)
+	    return 1;
+
+	c = (int *)zalloc(sizeof(int *));
+	
+	if(!c)
+	    return 1;
+
+	*c = next_cp;
+	addhashnode(zcurses_colorpairs, colorpair, (void *)c);
+    } 
+
+	fprintf(stderr, "%d\n", *c);
+
+    return (wcolor_set(w, *c, NULL) == ERR);
+}
+
+static void
+freecolornode(HashNode hn)
+{
+    int *i = (int *) hn;
+
+    zfree(i, sizeof(int));
+}
+
 /**/
 static int
 bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
@@ -180,6 +260,25 @@
 	if (!win_zero) {
 	    gettyinfo(&saved_tty_state);
 	    win_zero = initscr();
+	    if (start_color() != ERR) {
+		if(!zc_color_phase)
+		    zc_color_phase = 1;
+		zcurses_colorpairs = newhashtable(8, "zc_colorpairs", NULL);
+
+		zcurses_colorpairs->hash        = hasher;
+	        zcurses_colorpairs->emptytable  = emptyhashtable;
+	        zcurses_colorpairs->filltable   = NULL;
+		zcurses_colorpairs->cmpnodes    = strcmp;
+		zcurses_colorpairs->addnode     = addhashnode;
+		zcurses_colorpairs->getnode     = gethashnode2;
+		zcurses_colorpairs->getnode2    = gethashnode2;
+		zcurses_colorpairs->removenode  = removehashnode;
+		zcurses_colorpairs->disablenode = NULL;
+		zcurses_colorpairs->enablenode  = NULL;
+		zcurses_colorpairs->freenode    = freecolornode;
+		zcurses_colorpairs->printnode   = NULL;
+
+	    }
 	    gettyinfo(&curses_tty_state);
 	} else {
 	    settyinfo(&curses_tty_state);
@@ -427,6 +526,23 @@
 	}
 	return 0;
     }
+    if (OPT_ISSET(ops,'C')) {
+	LinkNode node;
+	ZCWin w;
+
+	if (!args[0] || !args[1] || !zc_color_phase)
+	    return 1;
+
+	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	if (node == NULL) {
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	    return 1;
+	}
+
+	w = (ZCWin)getdata(node);
+
+	return zcurses_colorset(w->win, args[1]);
+    }
 
     return 0;
 }
@@ -436,7 +552,7 @@
  */
 
 static struct builtin bintab[] = {
-    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "Aab:cd:eimrs", NULL),
+    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "Aab:Ccd:eimrs", NULL),
 };
 
 static struct features module_features = {
@@ -483,6 +599,7 @@
 cleanup_(Module m)
 {
     freelinklist(zcurses_windows, (FreeFunc) zcurses_free_window);
+    deletehashtable(zcurses_colorpairs);
     return setfeatureenables(m, &module_features, NULL);
 }
 

Re: PATCH: curses tweaks, maybe

[Peter Stephenson]

On Thu, 18 Oct 2007 16:40:16 -0400
Clint Adams <clint@zsh.org> wrote:
> On Wed, Oct 17, 2007 at 04:39:19PM +0100, Peter Stephenson wrote:
> > zcurses -C window black/red
> 
> I don't know why this doesn't work; perhaps another man page I'm
> misreading.

This fixes up some problems with hash table usage.  I hope there aren't
any leaks, it probably wants some looking over.  I've also used shorts
where the manual page says it needs them; this shouldn't actually be a
problem since presumably the prototypes are OK.

We may want a more informative message if we run out of color pairs.
Otherwise it seems to be working... I'm just using a trivially modified
script.

If I get a chance I may look at turning the options into names in a
table as suggested by Bart.  This will also give us a way of doing
better argument length and other checking.

I've also got a patch necessary for compilation with curses instead of
ncurses, but that's at work so will have to wait till Monday.


zmodload zsh/curses

zcurses -i
zcurses -a tw $(( LINES - 10 )) $(( COLUMNS - 20 )) 5 10
zcurses -b tw
zcurses -m tw 1 1
zcurses -c tw B
zcurses -c tw l
zcurses -c tw a
zcurses -c tw h
zcurses -r tw
zcurses -m tw 2 2
zcurses -s tw String
zcurses -m tw 3 3
zcurses -C tw blue/red
zcurses -A tw +bold +underline
zcurses -s tw BoLD
zcurses -r tw
sleep 5
zcurses -d tw

zcurses -e

Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.13
diff -u -r1.13 curses.c
--- Src/Modules/curses.c	19 Oct 2007 20:21:29 -0000	1.13
+++ Src/Modules/curses.c	20 Oct 2007 12:05:11 -0000
@@ -54,6 +54,12 @@
     int number;
 };
 
+struct colorpairnode {
+    struct hashnode node;
+    short colorpair;
+};
+typedef struct colorpairnode *Colorpairnode;
+
 static WINDOW *win_zero;
 static struct ttyinfo saved_tty_state;
 static struct ttyinfo curses_tty_state;
@@ -70,7 +76,8 @@
 #define ZCURSES_ATTRON 1
 #define ZCURSES_ATTROFF 2
 
-static int zc_errno, zc_color_phase=0, next_cp=0;
+static int zc_errno, zc_color_phase=0;
+static short next_cp=0;
 
 static const char *
 zcurses_strerror(int err)
@@ -172,7 +179,7 @@
     return 1;
 }
 
-static int
+static short
 zcurses_color(char *color)
 {
     struct zcurses_namenumberpair *zc;
@@ -191,19 +198,21 @@
 
     for(zc=(struct zcurses_namenumberpair *)zcurses_colors;zc->name;zc++)
 	if (!strcmp(color, zc->name)) {
-	    return zc->number;
+	    return (short)zc->number;
 	}
 
-    return -1;
+    return (short)-1;
 }
 
 static int
 zcurses_colorset(WINDOW *w, char *colorpair)
 {
     char *fg, *bg, *cp;
-    int *c, f, b;
+    short f, b;
+    Colorpairnode cpn;
 
-    if (zc_color_phase==1 || !(c = (int *) gethashnode(zcurses_colorpairs, colorpair))) {
+    if (zc_color_phase==1 ||
+	!(cpn = (Colorpairnode) gethashnode(zcurses_colorpairs, colorpair))) {
 	zc_color_phase = 2;
 	cp = ztrdup(colorpair);
 	fg = strtok(cp, "/");
@@ -229,26 +238,23 @@
 	if (init_pair(next_cp, f, b) == ERR)
 	    return 1;
 
-	c = (int *)zalloc(sizeof(int *));
+	cpn = (Colorpairnode)zalloc(sizeof(struct colorpairnode));
 	
-	if(!c)
+	if (!cpn)
 	    return 1;
 
-	*c = next_cp;
-	addhashnode(zcurses_colorpairs, colorpair, (void *)c);
-    } 
-
-	fprintf(stderr, "%d\n", *c);
+	cpn->colorpair = next_cp;
+	addhashnode(zcurses_colorpairs, ztrdup(colorpair), (void *)cpn);
+    }
 
-    return (wcolor_set(w, *c, NULL) == ERR);
+    return (wcolor_set(w, cpn->colorpair, NULL) == ERR);
 }
 
 static void
-freecolornode(HashNode hn)
+freecolorpairnode(HashNode hn)
 {
-    int *i = (int *) hn;
-
-    zfree(i, sizeof(int));
+    zsfree(hn->nam);
+    zfree(hn, sizeof(struct colorpairnode));
 }
 
 /**/
@@ -275,7 +281,7 @@
 		zcurses_colorpairs->removenode  = removehashnode;
 		zcurses_colorpairs->disablenode = NULL;
 		zcurses_colorpairs->enablenode  = NULL;
-		zcurses_colorpairs->freenode    = freecolornode;
+		zcurses_colorpairs->freenode    = freecolorpairnode;
 		zcurses_colorpairs->printnode   = NULL;
 
 	    }


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Sat, Oct 20, 2007 at 01:12:46PM +0100, Peter Stephenson wrote:
> If I get a chance I may look at turning the options into names in a
> table as suggested by Bart.  This will also give us a way of doing
> better argument length and other checking.

This needs the checking bit and a doc update.

zcurses init
zcurses addwin tw $(( LINES - 10 )) $(( COLUMNS - 20 )) 5 10
zcurses border tw
zcurses move tw 1 1
zcurses c tw B
zcurses c tw l
zcurses c tw a
zcurses c tw h
zcurses refresh tw
zcurses move tw 2 2
zcurses s tw String
zcurses move tw 3 3
zcurses attr tw +bold +underline
zcurses s tw BoLD
zcurses move tw 4 4
zcurses attr tw -bold -underline
zcurses color tw green/black
zcurses s tw Green
zcurses refresh tw
sleep 5
zcurses delwin tw
zcurses endwin

Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.14
diff -u -r1.14 curses.c
--- Src/Modules/curses.c	20 Oct 2007 12:15:17 -0000	1.14
+++ Src/Modules/curses.c	20 Oct 2007 13:33:44 -0000
@@ -60,6 +60,12 @@
 };
 typedef struct colorpairnode *Colorpairnode;
 
+struct zcurses_subcommand {
+    struct zcurses_namenumberpair nn;
+    int minargs;
+    int maxargs;
+};
+
 static WINDOW *win_zero;
 static struct ttyinfo saved_tty_state;
 static struct ttyinfo curses_tty_state;
@@ -76,6 +82,18 @@
 #define ZCURSES_ATTRON 1
 #define ZCURSES_ATTROFF 2
 
+#define ZCURSES_SC_INIT 1
+#define ZCURSES_SC_ADDWIN 2
+#define ZCURSES_SC_DELWIN 3
+#define ZCURSES_SC_REFRESH 4
+#define ZCURSES_SC_MOVE 5
+#define ZCURSES_SC_CHAR 6
+#define ZCURSES_SC_STRING 7
+#define ZCURSES_SC_BORDER 8
+#define ZCURSES_SC_ENDWIN 9
+#define ZCURSES_SC_ATTR 10
+#define ZCURSES_SC_COLOR 11
+
 static int zc_errno, zc_color_phase=0;
 static short next_cp=0;
 
@@ -261,8 +279,42 @@
 static int
 bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
 {
+    char **saargs;
+    struct zcurses_subcommand *zcsc;
+    int sc;
+
+    struct zcurses_subcommand scs[] = {
+	{{"init", ZCURSES_SC_INIT}, 0, 0},
+	{{"addwin", ZCURSES_SC_ADDWIN}, 5, 5},
+	{{"delwin", ZCURSES_SC_DELWIN}, 1, 1},
+	{{"refresh", ZCURSES_SC_REFRESH}, 0, 1},
+	{{"move", ZCURSES_SC_MOVE}, 3, 3},
+	{{"c", ZCURSES_SC_CHAR}, 2, 2},
+	{{"s", ZCURSES_SC_STRING}, 2, 2},
+	{{"border", ZCURSES_SC_BORDER}, 1, 5},
+	{{"endwin", ZCURSES_SC_ENDWIN}, 1, 1},
+	{{"attr", ZCURSES_SC_ATTR}, 2, 2},
+	{{"color", ZCURSES_SC_COLOR}, 2, 2},
+	{{NULL, -1}, 0, 0}
+    };
+
+    for(zcsc = scs; zcsc->nn.name; zcsc++) {
+	if(!strcmp(args[0], zcsc->nn.name))
+	    break;
+    }
+
+    sc = zcsc->nn.number;
+
+    if (sc == -1) {
+	zwarnnam(nam, "unknown subcommand: %s", args[0], 0);
+	return 1;
+    }
+
+    /* here would be a good place to validate number of args */
+    saargs = args + 1;
+
     /* Initialise curses */
-    if (OPT_ISSET(ops,'i')) {
+    if (sc == ZCURSES_SC_INIT) {
 	if (!win_zero) {
 	    gettyinfo(&saved_tty_state);
 	    win_zero = initscr();
@@ -272,8 +324,8 @@
 		zcurses_colorpairs = newhashtable(8, "zc_colorpairs", NULL);
 
 		zcurses_colorpairs->hash        = hasher;
-	        zcurses_colorpairs->emptytable  = emptyhashtable;
-	        zcurses_colorpairs->filltable   = NULL;
+		zcurses_colorpairs->emptytable  = emptyhashtable;
+		zcurses_colorpairs->filltable   = NULL;
 		zcurses_colorpairs->cmpnodes    = strcmp;
 		zcurses_colorpairs->addnode     = addhashnode;
 		zcurses_colorpairs->getnode     = gethashnode2;
@@ -290,27 +342,26 @@
 	    settyinfo(&curses_tty_state);
 	}
 	return 0;
-    }
-
-    if (OPT_ISSET(ops,'a')) {
+    } else
+    if (sc == ZCURSES_SC_ADDWIN) {
 	int nlines, ncols, begin_y, begin_x;
 	ZCWin w;
 
-	if (zcurses_validate_window(args[0], ZCURSES_UNUSED) == NULL && zc_errno) {
-	    zerrnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	if (zcurses_validate_window(saargs[0], ZCURSES_UNUSED) == NULL && zc_errno) {
+	    zerrnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
-	nlines = atoi(args[1]);
-	ncols = atoi(args[2]);
-	begin_y = atoi(args[3]);
-	begin_x = atoi(args[4]);
+	nlines = atoi(saargs[1]);
+	ncols = atoi(saargs[2]);
+	begin_y = atoi(saargs[3]);
+	begin_x = atoi(saargs[4]);
 
 	w = (ZCWin)zshcalloc(sizeof(struct zc_win));
 	if (!w)
 	    return 1;
 
-	w->name = ztrdup(args[0]);
+	w->name = ztrdup(saargs[0]);
 	w->win = newwin(nlines, ncols, begin_y, begin_x);
 
 	if (w->win == NULL) {
@@ -322,22 +373,21 @@
 	zinsertlinknode(zcurses_windows, lastnode(zcurses_windows), (void *)w);
 
 	return 0;
-    }
-
-    if (OPT_ISSET(ops,'d')) {
+    } else
+    if (sc == ZCURSES_SC_DELWIN) {
 	LinkNode node;
 	ZCWin w;
 
-	node = zcurses_validate_window(OPT_ARG(ops,'d'), ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), OPT_ARG(ops,'d'), 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
 	w = (ZCWin)getdata(node);
 
 	if (w == NULL) {
-	    zwarnnam(nam, "record for window `%s' is corrupt", OPT_ARG(ops, 'd'), 0);
+	    zwarnnam(nam, "record for window `%s' is corrupt", saargs[0], 0);
 	    return 1;
 	}
 	if (delwin(w->win)!=OK)
@@ -349,16 +399,15 @@
 	zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));
 
 	return 0;
-    }
-
-    if (OPT_ISSET(ops,'r')) {
-	if (args[0]) {
+    } else
+    if (sc == ZCURSES_SC_REFRESH) {
+	if (saargs[0]) {
 	    LinkNode node;
 	    ZCWin w;
 
-	    node = zcurses_validate_window(args[0], ZCURSES_USED);
+	    node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	    if (node == NULL) {
-		zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0],
+		zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0],
 			0);
 		return 1;
 	    }
@@ -371,21 +420,20 @@
 	{
 	    return (refresh() != OK) ? 1 : 0;
 	}
-    }
-
-    if (OPT_ISSET(ops,'m')) {
+    } else
+    if (sc == ZCURSES_SC_MOVE) {
 	int y, x;
 	LinkNode node;
 	ZCWin w;
 
-	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
-	y = atoi(args[1]);
-	x = atoi(args[2]);
+	y = atoi(saargs[1]);
+	x = atoi(saargs[2]);
 
 	w = (ZCWin)getdata(node);
 
@@ -393,9 +441,8 @@
 	    return 1;
 
 	return 0;
-    }
-
-    if (OPT_ISSET(ops,'c')) {
+    } else
+    if (sc == ZCURSES_SC_CHAR) {
 	LinkNode node;
 	ZCWin w;
 #ifdef HAVE_SETCCHAR
@@ -403,16 +450,16 @@
 	cchar_t cc;
 #endif
 
-	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
 	w = (ZCWin)getdata(node);
 
 #ifdef HAVE_SETCCHAR
-	if (mbrtowc(&c, args[1], MB_CUR_MAX, NULL) < 1)
+	if (mbrtowc(&c, saargs[1], MB_CUR_MAX, NULL) < 1)
 	    return 1;
 
 	if (setcchar(&cc, &c, A_NORMAL, 0, NULL)==ERR)
@@ -421,14 +468,13 @@
 	if (wadd_wch(w->win, &cc)!=OK)
 	    return 1;
 #else
-	if (waddch(w->win, (chtype)args[1][0])!=OK)
+	if (waddch(w->win, (chtype)saargs[1][0])!=OK)
 	    return 1;
 #endif
 
 	return 0;
-    }
-
-    if (OPT_ISSET(ops,'s')) {
+    } else
+    if (sc == ZCURSES_SC_STRING) {
 	LinkNode node;
 	ZCWin w;
 
@@ -436,10 +482,10 @@
 	int clen;
 	wint_t wc;
 	wchar_t *wstr, *wptr;
-	char *str = args[1];
+	char *str = saargs[1];
 #endif
 
-	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
 	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
 	    return 1;
@@ -462,19 +508,18 @@
 	    return 1;
 	}
 #else
-	if (waddstr(w->win, args[1])!=OK)
+	if (waddstr(w->win, saargs[1])!=OK)
 	    return 1;
 #endif
 	return 0;
-    }
-
-    if (OPT_ISSET(ops,'b')) {
+    } else
+    if (sc == ZCURSES_SC_BORDER) {
 	LinkNode node;
 	ZCWin w;
 
-	node = zcurses_validate_window(OPT_ARG(ops,'b'), ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), OPT_ARG(ops,'b'), 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
@@ -484,10 +529,9 @@
 	    return 1;
 
 	return 0;
-    }
-
+    } else
     /* Finish using curses */
-    if (OPT_ISSET(ops,'e')) {
+    if (sc == ZCURSES_SC_ENDWIN) {
 	if (win_zero) {
 	    endwin();
 	    /* Restore TTY as it was before zcurses -i */
@@ -500,24 +544,24 @@
 	    gettyinfo(&shttyinfo);
 	}
 	return 0;
-    }
-    if (OPT_ISSET(ops,'A')) {
+    } else
+    if (sc == ZCURSES_SC_ATTR) {
 	LinkNode node;
 	ZCWin w;
 	char **attrs;
 
-	if (!args[0])
+	if (!saargs[0])
 	    return 1;
 
-	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
 	w = (ZCWin)getdata(node);
 
-	for(attrs = args+1; *attrs; attrs++) {
+	for(attrs = saargs+1; *attrs; attrs++) {
 	    switch(*attrs[0]) {
 		case '-':
 		    zcurses_attribute(w->win, (*attrs)+1, ZCURSES_ATTROFF);
@@ -531,23 +575,23 @@
 	    }
 	}
 	return 0;
-    }
-    if (OPT_ISSET(ops,'C')) {
+    } else
+    if (sc == ZCURSES_SC_COLOR) {
 	LinkNode node;
 	ZCWin w;
 
-	if (!args[0] || !args[1] || !zc_color_phase)
+	if (!saargs[0] || !saargs[1] || !zc_color_phase)
 	    return 1;
 
-	node = zcurses_validate_window(args[0], ZCURSES_USED);
+	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
 	    return 1;
 	}
 
 	w = (ZCWin)getdata(node);
 
-	return zcurses_colorset(w->win, args[1]);
+	return zcurses_colorset(w->win, saargs[1]);
     }
 
     return 0;
@@ -558,7 +602,7 @@
  */
 
 static struct builtin bintab[] = {
-    BUILTIN("zcurses", 0, bin_zcurses, 0, 5, 0, "Aab:Ccd:eimrs", NULL),
+    BUILTIN("zcurses", 0, bin_zcurses, 1, 6, 0, "", NULL),
 };
 
 static struct features module_features = {

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Sat, Oct 20, 2007 at 09:37:09AM -0400, Clint Adams wrote:
> This needs the checking bit and a doc update.

Index: Doc/Zsh/mod_curses.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_curses.yo,v
retrieving revision 1.6
diff -u -r1.6 mod_curses.yo
--- Doc/Zsh/mod_curses.yo	17 Oct 2007 20:27:26 -0000	1.6
+++ Doc/Zsh/mod_curses.yo	21 Oct 2007 19:49:18 -0000
@@ -6,42 +6,42 @@
 startitem()
 findex(zcurses)
 cindex(windows, curses)
-xitem(tt(zcurses) tt(-i))
-xitem(tt(zcurses) tt(-e))
-xitem(tt(zcurses) tt(-a) var(targetwin) var(nlines) var(ncols) var(begin_y) var(begin_x) )
-xitem(tt(zcurses) tt(-d) var(targetwin) )
-xitem(tt(zcurses) tt(-r) [ var(targetwin) ] )
-xitem(tt(zcurses) tt(-m) var(targetwin) var(new_y) var(new_x) )
-xitem(tt(zcurses) tt(-c) var(targetwin) var(character) )
-xitem(tt(zcurses) tt(-s) var(targetwin) var(string) )
-xitem(tt(zcurses) tt(-b) var(targetwin) var(border) )(
-item(tt(zcurses) tt(-A) var(targetwin) var({+/-}attribute) [var({+/-}attribute)] [...])(
+xitem(tt(zcurses) tt(init))
+xitem(tt(zcurses) tt(endwin))
+xitem(tt(zcurses) tt(addwin) var(targetwin) var(nlines) var(ncols) var(begin_y) var(begin_x) )
+xitem(tt(zcurses) tt(delwin) var(targetwin) )
+xitem(tt(zcurses) tt(refresh) [ var(targetwin) ] )
+xitem(tt(zcurses) tt(move) var(targetwin) var(new_y) var(new_x) )
+xitem(tt(zcurses) tt(c) var(targetwin) var(character) )
+xitem(tt(zcurses) tt(s) var(targetwin) var(string) )
+xitem(tt(zcurses) tt(border) var(targetwin) var(border) )(
+item(tt(zcurses) tt(addwin) var(targetwin) var({+/-}attribute) [var({+/-}attribute)] [...])(
 Manipulate curses windows.  All uses of this command should be
-bracketed by `tt(zcurses -i)' to initialise use of curses, and
-`tt(zcurses -e)' to end it; omitting `tt(zcurses -e)' can cause
+bracketed by `tt(zcurses init)' to initialise use of curses, and
+`tt(zcurses endwin)' to end it; omitting `tt(zcurses endwin)' can cause
 the terminal to be in an unwanted state.
 
-With tt(-a), create a window with var(nlines) lines and var(ncols) columns.
+With tt(addwin), create a window with var(nlines) lines and var(ncols) columns.
 Its upper left corner will be placed at row var(begin_y) and column
 var(begin_x) of the screen.  var(targetwin) is a string and refers
 to the name of a window that is not currently assigned.
 
-Use tt(-d) to delete a window created with tt(-a).
+Use tt(delwin) to delete a window created with tt(addwin).
 
-The tt(-r) command will refresh window var(targetwin); this is necessary to
+The tt(refresh) command will refresh window var(targetwin); this is necessary to
 make any pending changes (such as characters you have prepared for output
-with tt(-c)) visible on the screen.  If no argument is given,
+with tt(c)) visible on the screen.  If no argument is given,
 all windows are refreshed; this is necessary after deleting a window.
 
-tt(-m) moves the cursor position in var(targetwin) to new coordinates
+tt(move) moves the cursor position in var(targetwin) to new coordinates
 var(new_y) and var(new_x).
 
-Outputting characters and strings are achieved by tt(-c) and tt(-s)
+Outputting characters and strings are achieved by tt(c) and tt(s)
 respectively.
 
-To draw a border around window var(targetwin), use tt(-b).
+To draw a border around window var(targetwin), use tt(border).
 
-tt(-A) will set var(targetwin)'s attributes for any successive character
+tt(addwin) will set var(targetwin)'s attributes for any successive character
 output.  Each var(attribute) given on the line should be prepended by a
 tt(+) to set or a tt(-) to unset that attribute.  The attributes supported
 are tt(blink), tt(bold), tt(dim), tt(reverse), tt(standout), and

Re: PATCH: curses tweaks, maybe

[Clint Adams]

On Sat, Oct 20, 2007 at 09:37:09AM -0400, Clint Adams wrote:
> This needs the checking bit and a doc update.

This is probably not the most efficient.

Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.15
diff -u -r1.15 curses.c
--- Src/Modules/curses.c	20 Oct 2007 13:40:02 -0000	1.15
+++ Src/Modules/curses.c	21 Oct 2007 21:08:54 -0000
@@ -281,7 +281,7 @@
 {
     char **saargs;
     struct zcurses_subcommand *zcsc;
-    int sc;
+    int sc, num_args;
 
     struct zcurses_subcommand scs[] = {
 	{{"init", ZCURSES_SC_INIT}, 0, 0},
@@ -310,7 +310,13 @@
 	return 1;
     }
 
-    /* here would be a good place to validate number of args */
+    saargs = args;
+    while (*saargs++);
+    num_args = saargs - (args + 2);
+
+    if (num_args < zcsc->minargs || num_args > zcsc->maxargs)
+	return 1;
+
     saargs = args + 1;
 
     /* Initialise curses */