PATCH: zcurses stuff

PATCH: zcurses stuff

[Peter Stephenson]

1. Wrong argument counts for attr and endwin
2. More verbose errors for wrong number of arguments
3. One cchar_t should be a wchar_t
4. Now we have a command table, it's neater to dispatch straight
   to subcommands.
5. attr didn't pass back the status from actually setting attributes.

Other things I've thought about, but not done:
1. I think I prefer "end" to "endwin".  As I said, curses naming
   is horrible and "endwin" to me suggests it applies to a single
   window (indeed, what else could it logically mean?)
2. I think I prefer "char" and "string" to "c" and "s", which are
   the names of the subcommands internally anyway.
3. I'm not sure we actually need both "attr" and "color"; we
   can easily discriminate between types and as far as the user is
   concerned it's natural to set them in a single command,
     zcurses attr +bold red/black
   Also, I think we could assume the "+" if not present; other
   bits of the shell do this.
4. I think we can be more verbose with errors in places, for
   example if colo[u]rs or attributes aren't found.  The fact that
   it messes up the screen is neither here nor there if the result
   didn't work anyway.

By the way, I've adapted the test function so that it always tidies up:


zmodload zsh/curses

{
  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
} always {
  zcurses endwin
}


Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.16
diff -u -r1.16 curses.c
--- Src/Modules/curses.c	21 Oct 2007 21:16:07 -0000	1.16
+++ Src/Modules/curses.c	23 Oct 2007 20:43:51 -0000
@@ -60,8 +60,10 @@
 };
 typedef struct colorpairnode *Colorpairnode;
 
+typedef int (*zccmd_t)(const char *nam, char **args);
 struct zcurses_subcommand {
-    struct zcurses_namenumberpair nn;
+    const char *name;
+    zccmd_t cmd;
     int minargs;
     int maxargs;
 };
@@ -110,7 +112,7 @@
 }
 
 static LinkNode
-zcurses_getwindowbyname(char *name)
+zcurses_getwindowbyname(const char *name)
 {
     LinkNode node;
     ZCWin w;
@@ -198,7 +200,7 @@
 }
 
 static short
-zcurses_color(char *color)
+zcurses_color(const char *color)
 {
     struct zcurses_namenumberpair *zc;
 
@@ -275,332 +277,385 @@
     zfree(hn, sizeof(struct colorpairnode));
 }
 
-/**/
+
+/*************
+ * Subcommands
+ *************/
+
 static int
-bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
+zccmd_init(const char *nam, char **args)
 {
-    char **saargs;
-    struct zcurses_subcommand *zcsc;
-    int sc, num_args;
+    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    = freecolorpairnode;
+	    zcurses_colorpairs->printnode   = NULL;
+
+	}
+	gettyinfo(&curses_tty_state);
+    } else {
+	settyinfo(&curses_tty_state);
+    }
+    return 0;
+}
 
-    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;
+static int
+zccmd_addwin(const char *nam, char **args)
+{
+    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);
+	return 1;
     }
 
-    sc = zcsc->nn.number;
+    nlines = atoi(args[1]);
+    ncols = atoi(args[2]);
+    begin_y = atoi(args[3]);
+    begin_x = atoi(args[4]);
 
-    if (sc == -1) {
-	zwarnnam(nam, "unknown subcommand: %s", args[0], 0);
+    w = (ZCWin)zshcalloc(sizeof(struct zc_win));
+    if (!w)
 	return 1;
-    }
 
-    saargs = args;
-    while (*saargs++);
-    num_args = saargs - (args + 2);
+    w->name = ztrdup(args[0]);
+    w->win = newwin(nlines, ncols, begin_y, begin_x);
 
-    if (num_args < zcsc->minargs || num_args > zcsc->maxargs)
+    if (w->win == NULL) {
+	zsfree(w->name);
+	free(w);
 	return 1;
+    }
 
-    saargs = args + 1;
+    zinsertlinknode(zcurses_windows, lastnode(zcurses_windows), (void *)w);
 
-    /* Initialise curses */
-    if (sc == ZCURSES_SC_INIT) {
-	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    = freecolorpairnode;
-		zcurses_colorpairs->printnode   = NULL;
+    return 0;
+}
 
-	    }
-	    gettyinfo(&curses_tty_state);
-	} else {
-	    settyinfo(&curses_tty_state);
-	}
-	return 0;
-    } else
-    if (sc == ZCURSES_SC_ADDWIN) {
-	int nlines, ncols, begin_y, begin_x;
-	ZCWin w;
+static int
+zccmd_delwin(const char *nam, char **args)
+{
+    LinkNode node;
+    ZCWin w;
 
-	if (zcurses_validate_window(saargs[0], ZCURSES_UNUSED) == NULL && zc_errno) {
-	    zerrnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 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;
+    }
 
-	nlines = atoi(saargs[1]);
-	ncols = atoi(saargs[2]);
-	begin_y = atoi(saargs[3]);
-	begin_x = atoi(saargs[4]);
+    w = (ZCWin)getdata(node);
 
-	w = (ZCWin)zshcalloc(sizeof(struct zc_win));
-	if (!w)
-	    return 1;
+    if (w == NULL) {
+	zwarnnam(nam, "record for window `%s' is corrupt", args[0], 0);
+	return 1;
+    }
+    if (delwin(w->win)!=OK)
+	return 1;
 
-	w->name = ztrdup(saargs[0]);
-	w->win = newwin(nlines, ncols, begin_y, begin_x);
+    if (w->name)
+	zsfree(w->name);
 
-	if (w->win == NULL) {
-	    zsfree(w->name);
-	    free(w);
-	    return 1;
-	}
+    zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));
 
-	zinsertlinknode(zcurses_windows, lastnode(zcurses_windows), (void *)w);
+    return 0;
+}
 
-	return 0;
-    } else
-    if (sc == ZCURSES_SC_DELWIN) {
+static int
+zccmd_refresh(const char *nam, char **args)
+{
+    if (args[0]) {
 	LinkNode node;
 	ZCWin w;
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
+	node = zcurses_validate_window(args[0], ZCURSES_USED);
 	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
+	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0],
+		     0);
 	    return 1;
 	}
 
 	w = (ZCWin)getdata(node);
 
-	if (w == NULL) {
-	    zwarnnam(nam, "record for window `%s' is corrupt", saargs[0], 0);
-	    return 1;
-	}
-	if (delwin(w->win)!=OK)
-	    return 1;
-
-	if (w->name)
-	    zsfree(w->name);
+	return (wrefresh(w->win)!=OK) ? 1 : 0;
+    }
+    else
+    {
+	return (refresh() != OK) ? 1 : 0;
+    }
+}
 
-	zfree((ZCWin)remnode(zcurses_windows, node), sizeof(struct zc_win));
 
-	return 0;
-    } else
-    if (sc == ZCURSES_SC_REFRESH) {
-	if (saargs[0]) {
-	    LinkNode node;
-	    ZCWin w;
-
-	    node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	    if (node == NULL) {
-		zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0],
-			0);
-		return 1;
-	    }
+static int
+zccmd_move(const char *nam, char **args)
+{
+    int y, x;
+    LinkNode node;
+    ZCWin w;
 
-	    w = (ZCWin)getdata(node);
+    node = zcurses_validate_window(args[0], ZCURSES_USED);
+    if (node == NULL) {
+	zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 0);
+	return 1;
+    }
 
-	    return (wrefresh(w->win)!=OK) ? 1 : 0;
-	}
-	else
-	{
-	    return (refresh() != OK) ? 1 : 0;
-	}
-    } else
-    if (sc == ZCURSES_SC_MOVE) {
-	int y, x;
-	LinkNode node;
-	ZCWin w;
+    y = atoi(args[1]);
+    x = atoi(args[2]);
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
-	    return 1;
-	}
+    w = (ZCWin)getdata(node);
 
-	y = atoi(saargs[1]);
-	x = atoi(saargs[2]);
+    if (wmove(w->win, y, x)!=OK)
+	return 1;
 
-	w = (ZCWin)getdata(node);
+    return 0;
+}
 
-	if (wmove(w->win, y, x)!=OK)
-	    return 1;
 
-	return 0;
-    } else
-    if (sc == ZCURSES_SC_CHAR) {
-	LinkNode node;
-	ZCWin w;
+static int
+zccmd_char(const char *nam, char **args)
+{
+    LinkNode node;
+    ZCWin w;
 #ifdef HAVE_SETCCHAR
-	wchar_t c;
-	cchar_t cc;
+    wchar_t c;
+    cchar_t cc;
 #endif
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 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);
+    w = (ZCWin)getdata(node);
 
 #ifdef HAVE_SETCCHAR
-	if (mbrtowc(&c, saargs[1], MB_CUR_MAX, NULL) < 1)
-	    return 1;
+    if (mbrtowc(&c, args[1], MB_CUR_MAX, NULL) < 1)
+	return 1;
 
-	if (setcchar(&cc, &c, A_NORMAL, 0, NULL)==ERR)
-	    return 1;
+    if (setcchar(&cc, &c, A_NORMAL, 0, NULL)==ERR)
+	return 1;
 
-	if (wadd_wch(w->win, &cc)!=OK)
-	    return 1;
+    if (wadd_wch(w->win, &cc)!=OK)
+	return 1;
 #else
-	if (waddch(w->win, (chtype)saargs[1][0])!=OK)
-	    return 1;
+    if (waddch(w->win, (chtype)args[1][0])!=OK)
+	return 1;
 #endif
 
-	return 0;
-    } else
-    if (sc == ZCURSES_SC_STRING) {
-	LinkNode node;
-	ZCWin w;
+    return 0;
+}
+
+
+static int
+zccmd_string(const char *nam, char **args)
+{
+    LinkNode node;
+    ZCWin w;
 
 #ifdef HAVE_WADDWSTR
-	int clen;
-	wint_t wc;
-	wchar_t *wstr, *wptr;
-	char *str = saargs[1];
+    int clen;
+    wint_t wc;
+    wchar_t *wstr, *wptr;
+    char *str = args[1];
 #endif
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), args[0], 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);
+    w = (ZCWin)getdata(node);
 
 #ifdef HAVE_WADDWSTR
-	mb_metacharinit();
-	wptr = wstr = zhalloc((strlen(str)+1) * sizeof(cchar_t));
+    mb_metacharinit();
+    wptr = wstr = zhalloc((strlen(str)+1) * sizeof(wchar_t));
 
-	while (*str && (clen = mb_metacharlenconv(str, &wc))) {
-	    str += clen;
-	    if (wc == WEOF) /* TODO: replace with space? nicen? */
-		continue;
-	    *wptr++ = wc;
-	}
-	*wptr++ = L'\0';
-	if (waddwstr(w->win, wstr)!=OK) {
-	    return 1;
-	}
+    while (*str && (clen = mb_metacharlenconv(str, &wc))) {
+	str += clen;
+	if (wc == WEOF) /* TODO: replace with space? nicen? */
+	    continue;
+	*wptr++ = wc;
+    }
+    *wptr++ = L'\0';
+    if (waddwstr(w->win, wstr)!=OK) {
+	return 1;
+    }
 #else
-	if (waddstr(w->win, saargs[1])!=OK)
-	    return 1;
+    if (waddstr(w->win, args[1])!=OK)
+	return 1;
 #endif
-	return 0;
-    } else
-    if (sc == ZCURSES_SC_BORDER) {
-	LinkNode node;
-	ZCWin w;
+    return 0;
+}
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
-	    return 1;
-	}
 
-	w = (ZCWin)getdata(node);
+static int
+zccmd_border(const char *nam, char **args)
+{
+    LinkNode node;
+    ZCWin w;
 
-	if (wborder(w->win, 0, 0, 0, 0, 0, 0, 0, 0)!=OK)
-	    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;
+    }
 
-	return 0;
-    } else
-    /* Finish using curses */
-    if (sc == ZCURSES_SC_ENDWIN) {
-	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;
-    } else
-    if (sc == ZCURSES_SC_ATTR) {
-	LinkNode node;
-	ZCWin w;
-	char **attrs;
+    w = (ZCWin)getdata(node);
 
-	if (!saargs[0])
-	    return 1;
+    if (wborder(w->win, 0, 0, 0, 0, 0, 0, 0, 0)!=OK)
+	return 1;
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
-	    return 1;
-	}
+    return 0;
+}
 
-	w = (ZCWin)getdata(node);
 
-	for(attrs = saargs+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;
-    } else
-    if (sc == ZCURSES_SC_COLOR) {
-	LinkNode node;
-	ZCWin w;
+static int
+zccmd_endwin(const char *nam, char **args)
+{
+    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;
+}
 
-	if (!saargs[0] || !saargs[1] || !zc_color_phase)
-	    return 1;
 
-	node = zcurses_validate_window(saargs[0], ZCURSES_USED);
-	if (node == NULL) {
-	    zwarnnam(nam, "%s: %s", zcurses_strerror(zc_errno), saargs[0], 0);
-	    return 1;
+static int
+zccmd_attr(const char *nam, char **args)
+{
+    LinkNode node;
+    ZCWin w;
+    char **attrs;
+    int ret = 0;
+
+    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 '-':
+	    if (zcurses_attribute(w->win, (*attrs)+1, ZCURSES_ATTROFF))
+		ret = 1;
+	    break;
+	case '+':
+	    if (zcurses_attribute(w->win, (*attrs)+1, ZCURSES_ATTRON))
+		ret = 1;
+	    break;
+	default:
+	    /* maybe a bad idea to spew warnings here */
+	    break;
 	}
+    }
+    return ret;
+}
 
-	w = (ZCWin)getdata(node);
 
-	return zcurses_colorset(w->win, saargs[1]);
+static int
+zccmd_color(const char *nam, char **args)
+{
+    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;
     }
 
-    return 0;
+    w = (ZCWin)getdata(node);
+
+    return zcurses_colorset(w->win, args[1]);
+}
+
+
+/*********************
+  Main builtin handler
+ *********************/
+
+/**/
+static int
+bin_zcurses(char *nam, char **args, Options ops, UNUSED(int func))
+{
+    char **saargs;
+    struct zcurses_subcommand *zcsc;
+    int num_args;
+
+    struct zcurses_subcommand scs[] = {
+	{"init", zccmd_init, 0, 0},
+	{"addwin", zccmd_addwin, 5, 5},
+	{"delwin", zccmd_delwin, 1, 1},
+	{"refresh", zccmd_refresh, 0, 1},
+	{"move", zccmd_move, 3, 3},
+	{"c", zccmd_char, 2, 2},
+	{"s", zccmd_string, 2, 2},
+	{"border", zccmd_border, 1, 5},
+	{"endwin", zccmd_endwin, 0, 0},
+	{"attr", zccmd_attr, 2, -1},
+	{"color", zccmd_color, 2, 2},
+	{NULL, (zccmd_t)0, 0, 0}
+    };
+
+    for(zcsc = scs; zcsc->name; zcsc++) {
+	if(!strcmp(args[0], zcsc->name))
+	    break;
+    }
+
+    if (zcsc->name == NULL) {
+	zwarnnam(nam, "unknown subcommand: %s", args[0]);
+	return 1;
+    }
+
+    saargs = args;
+    while (*saargs++);
+    num_args = saargs - (args + 2);
+
+    if (num_args < zcsc->minargs) {
+	zwarnnam(nam, "too few arguments for subcommand: %s", args[0]);
+	return 1;
+    } else if (zcsc->maxargs >= 0 && num_args > zcsc->maxargs) {
+	zwarnnam(nam, "too may arguments for subcommand: %s", args[0]);
+	return 1;
+    }
+
+    return zcsc->cmd(nam, args+1);
 }
 
 /*


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

Re: PATCH: zcurses stuff

[Clint Adams]

On Tue, Oct 23, 2007 at 09:45:55PM +0100, Peter Stephenson wrote:
> Other things I've thought about, but not done:
> 1. I think I prefer "end" to "endwin".  As I said, curses naming
>    is horrible and "endwin" to me suggests it applies to a single
>    window (indeed, what else could it logically mean?)
> 2. I think I prefer "char" and "string" to "c" and "s", which are
>    the names of the subcommands internally anyway.

I'm still wondering if we should handle both char and string with
a single subcommand.

> 3. I'm not sure we actually need both "attr" and "color"; we
>    can easily discriminate between types and as far as the user is
>    concerned it's natural to set them in a single command,
>      zcurses attr +bold red/black
>    Also, I think we could assume the "+" if not present; other
>    bits of the shell do this.

Sounds fine to me.

> 4. I think we can be more verbose with errors in places, for
>    example if colo[u]rs or attributes aren't found.  The fact that
>    it messes up the screen is neither here nor there if the result
>    didn't work anyway.

We should expose some kind of interface for the caller to determine
which colo[u]rs (if any) and attributes (when/if we start supporting
optional ones) are available before trying to use them then.

Index: Doc/Zsh/mod_curses.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_curses.yo,v
retrieving revision 1.7
diff -u -r1.7 mod_curses.yo
--- Doc/Zsh/mod_curses.yo	21 Oct 2007 19:53:44 -0000	1.7
+++ Doc/Zsh/mod_curses.yo	23 Oct 2007 21:03:06 -0000
@@ -7,18 +7,18 @@
 findex(zcurses)
 cindex(windows, curses)
 xitem(tt(zcurses) tt(init))
-xitem(tt(zcurses) tt(endwin))
+xitem(tt(zcurses) tt(end))
 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(char) var(targetwin) var(character) )
+xitem(tt(zcurses) tt(string) 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 init)' to initialise use of curses, and
-`tt(zcurses endwin)' to end it; omitting `tt(zcurses endwin)' can cause
+`tt(zcurses end)' to end it; omitting `tt(zcurses end)' can cause
 the terminal to be in an unwanted state.
 
 With tt(addwin), create a window with var(nlines) lines and var(ncols) columns.
@@ -30,13 +30,13 @@
 
 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(char)) visible on the screen.  If no argument is given,
 all windows are refreshed; this is necessary after deleting a window.
 
 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(char) and tt(string)
 respectively.
 
 To draw a border around window var(targetwin), use tt(border).
Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.18
diff -u -r1.18 curses.c
--- Src/Modules/curses.c	23 Oct 2007 20:55:42 -0000	1.18
+++ Src/Modules/curses.c	23 Oct 2007 21:03:07 -0000
@@ -612,10 +612,10 @@
 	{"delwin", zccmd_delwin, 1, 1},
 	{"refresh", zccmd_refresh, 0, 1},
 	{"move", zccmd_move, 3, 3},
-	{"c", zccmd_char, 2, 2},
-	{"s", zccmd_string, 2, 2},
+	{"char", zccmd_char, 2, 2},
+	{"string", zccmd_string, 2, 2},
 	{"border", zccmd_border, 1, 1},
-	{"endwin", zccmd_endwin, 0, 0},
+	{"end", zccmd_endwin, 0, 0},
 	{"attr", zccmd_attr, 2, -1},
 	{"color", zccmd_color, 2, 2},
 	{NULL, (zccmd_t)0, 0, 0}

Re: PATCH: zcurses stuff

[Bart Schaefer]

On Oct 23,  9:45pm, Peter Stephenson wrote:
}
} 1. I think I prefer "end" to "endwin".  As I said, curses naming
}    is horrible and "endwin" to me suggests it applies to a single
}    window (indeed, what else could it logically mean?)

"End win"dowing system.

Re: PATCH: zcurses stuff

[Matthew Wozniski]

On Tue, Oct 23, 2007 at 05:52:55PM -0700, Bart Schaefer wrote:
> On Oct 23,  9:45pm, Peter Stephenson wrote:
> }
> } 1. I think I prefer "end" to "endwin".  As I said, curses naming
> }    is horrible and "endwin" to me suggests it applies to a single
> }    window (indeed, what else could it logically mean?)
> 
> "End win"dowing system.

FWIW, I agree with PWS.  endwin sounds like it should apply to
a single window, and end is a better name if we don't plan on keeping
the same names as curses.

~Matt

Re: PATCH: zcurses stuff

[Bart Schaefer]

On Oct 23,  9:06pm, Matthew Wozniski wrote:
} Subject: Re: PATCH: zcurses stuff
}
} On Tue, Oct 23, 2007 at 05:52:55PM -0700, Bart Schaefer wrote:
} > On Oct 23,  9:45pm, Peter Stephenson wrote:
} > }
} > } 1. I think I prefer "end" to "endwin".  As I said, curses naming
} > }    is horrible and "endwin" to me suggests it applies to a single
} > }    window (indeed, what else could it logically mean?)
} > 
} > "End win"dowing system.
} 
} FWIW, I agree with PWS.

Oh, I agree with him, too.  I was just answering the question.

Re: PATCH: zcurses stuff

[Peter Stephenson]

Clint Adams wrote:
> On Tue, Oct 23, 2007 at 09:45:55PM +0100, Peter Stephenson wrote:
> > 2. I think I prefer "char" and "string" to "c" and "s", which are
> >    the names of the subcommands internally anyway.
> 
> I'm still wondering if we should handle both char and string with
> a single subcommand.

Yes, that occurred to me too; we could call it "text".  I'm not sure if
we then need to handle single characters differently from strings.  If
we do, we need to be careful to see if it's a multibyte character first.

> We should expose some kind of interface for the caller to determine
> which colo[u]rs (if any) and attributes (when/if we start supporting
> optional ones) are available before trying to use them then.

We probably need other forms of querying, too.  It would be sensible to
provide a list of windows.  These could be special arrays,
zcurses_colors, zcurses_attrs, zcurses_windows?  We've traditionally had
a -l option for listing, but that doesn't seem to make sense here since
this is a purely programmatic interface.

While I'm here, here's the patch to make things work with just curses.h
and not ncurses.h.  The .mdd file already tests for the existence of
curses.h, so (supposedly) all we have to worry about is whether some
ancient versions of curses don't support even all the narrow character
features.

Index: configure.ac
===================================================================
RCS file: /cvsroot/zsh/zsh/configure.ac,v
retrieving revision 1.72
diff -u -r1.72 configure.ac
--- configure.ac	18 Oct 2007 08:29:33 -0000	1.72
+++ configure.ac	24 Oct 2007 08:42:37 -0000
@@ -557,7 +557,7 @@
 		 unistd.h sys/capability.h \
 		 utmp.h utmpx.h sys/types.h pwd.h grp.h poll.h sys/mman.h \
 		 netinet/in_systm.h pcre.h langinfo.h wchar.h stddef.h \
-		 sys/stropts.h iconv.h)
+		 sys/stropts.h iconv.h ncurses.h)
 if test x$dynamic = xyes; then
   AC_CHECK_HEADERS(dlfcn.h)
   AC_CHECK_HEADERS(dl.h)
Index: Src/Modules/curses.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/curses.c,v
retrieving revision 1.19
diff -u -r1.19 curses.c
--- Src/Modules/curses.c	23 Oct 2007 21:16:03 -0000	1.19
+++ Src/Modules/curses.c	24 Oct 2007 08:42:41 -0000
@@ -29,7 +29,17 @@
 
 #define _XOPEN_SOURCE_EXTENDED 1
 
-#include <ncurses.h>
+#include "curses.mdh"
+#include "curses.pro"
+
+#ifdef HAVE_NCURSES_H
+# include <ncurses.h>
+#else
+# ifdef HAVE_CURSES_H
+#  include <curses.h>
+# endif
+#endif
+
 #ifndef MULTIBYTE_SUPPORT
 # undef HAVE_SETCCHAR
 # undef HAVE_WADDWSTR
@@ -41,9 +51,6 @@
 
 #include <stdio.h>
 
-#include "curses.mdh"
-#include "curses.pro"
-
 typedef struct zc_win {
     WINDOW *win;
     char *name;


-- 
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