Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: PATCH: options for cd and other animals



Peter Stephenson wrote:
> Here's a tenative patch to rationalise the option handling for cd, as
> well as generally making the code more sane.

One thing I've already noticed is that I've been unnecessarily brutal
with rejecting numeric arguments when they look like options,
e.g. `print -6'.

This version reverses the assumption; you have to set BINF_KEEPNUM if
you want a command to treat numbers as options, whether or not it
actually handles them.  No commands currently do this, which corresponds
roughly with the old behaviour --- for some reason, commands which
accepted a `+', which means typeset-style commands plus `alias', also
treated -<num> as an (invalid) option.  This may be intentional; if
anyone knows it is, we can add BINF_KEEPNUM to those commands.  (This
might be the safest thing to do anyway.)

Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.51
diff -u -r1.51 builtins.yo
--- Doc/Zsh/builtins.yo	27 Aug 2002 21:10:34 -0000	1.51
+++ Doc/Zsh/builtins.yo	2 Sep 2002 13:16:31 -0000
@@ -772,9 +772,9 @@
 pindex(PUSHD_MINUS, use of)
 pindex(CDABLE_VARS, use of)
 pindex(PUSHD_SILENT, use of)
-xitem(tt(pushd) [ var(arg) ])
-xitem(tt(pushd) var(old) var(new))
-item(tt(pushd) {tt(PLUS())|tt(-)}var(n))(
+xitem(tt(pushd) [ tt(-sLP) ] [ var(arg) ])
+xitem(tt(pushd) [ tt(-sLP) ] var(old) var(new))
+item(tt(pushd) [ tt(-sLP) ] {tt(PLUS())|tt(-)}var(n))(
 Change the current directory, and push the old current directory
 onto the directory stack.  In the first form, change the
 current directory to var(arg).
@@ -795,6 +795,9 @@
 
 If the option tt(PUSHD_SILENT) is not set, the directory
 stack will be printed after a tt(pushd) is performed.
+
+The options tt(-s), tt(-L) and tt(-P) have the same meanings as for the
+tt(cd) builtin.
 )
 findex(pushln)
 item(tt(pushln) [ var(arg) ... ])(
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.82
diff -u -r1.82 builtin.c
--- Src/builtin.c	27 Aug 2002 21:10:34 -0000	1.82
+++ Src/builtin.c	2 Sep 2002 13:16:31 -0000
@@ -50,14 +50,14 @@
     BUILTIN("bg", 0, bin_fg, 0, -1, BIN_BG, NULL, NULL),
     BUILTIN("break", BINF_PSPECIAL, bin_break, 0, 1, BIN_BREAK, NULL, NULL),
     BUILTIN("bye", 0, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
-    BUILTIN("cd", 0, bin_cd, 0, 2, BIN_CD, NULL, NULL),
-    BUILTIN("chdir", 0, bin_cd, 0, 2, BIN_CD, NULL, NULL),
+    BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
+    BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "sPL", NULL),
     BUILTIN("continue", BINF_PSPECIAL, bin_break, 0, 1, BIN_CONTINUE, NULL, NULL),
     BUILTIN("declare", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL, bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%lprtux", NULL),
     BUILTIN("dirs", 0, bin_dirs, 0, -1, 0, "clpv", NULL),
     BUILTIN("disable", 0, bin_enable, 0, -1, BIN_DISABLE, "afmr", NULL),
     BUILTIN("disown", 0, bin_fg, 0, -1, BIN_DISOWN, NULL, NULL),
-    BUILTIN("echo", BINF_PRINTOPTS | BINF_ECHOPTS, bin_print, 0, -1, BIN_ECHO, "neE", "-"),
+    BUILTIN("echo", BINF_PRINTOPTS | BINF_SKIPINVALID, bin_print, 0, -1, BIN_ECHO, "neE", "-"),
     BUILTIN("emulate", 0, bin_emulate, 1, 1, 0, "LR", NULL),
     BUILTIN("enable", 0, bin_enable, 0, -1, BIN_ENABLE, "afmr", NULL),
     BUILTIN("eval", BINF_PSPECIAL, bin_eval, 0, -1, BIN_EVAL, NULL, NULL),
@@ -99,10 +99,10 @@
     BUILTIN("patdebug", 0, bin_patdebug, 1, -1, 0, "p", NULL),
 #endif
 
-    BUILTIN("popd", 0, bin_cd, 0, 2, BIN_POPD, NULL, NULL),
+    BUILTIN("popd", 0, bin_cd, 0, 1, BIN_POPD, NULL, NULL),
     BUILTIN("print", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, "RDPbnrsf:lzNu:pioOcm-", NULL),
     BUILTIN("printf", 0, bin_print, 1, -1, BIN_PRINTF, NULL, NULL),
-    BUILTIN("pushd", 0, bin_cd, 0, 2, BIN_PUSHD, NULL, NULL),
+    BUILTIN("pushd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_PUSHD, "sPL", NULL),
     BUILTIN("pushln", BINF_PRINTOPTS, bin_print, 0, -1, BIN_PRINT, NULL, "-nz"),
     BUILTIN("pwd", 0, bin_pwd, 0, 0, 0, "rLP", NULL),
     BUILTIN("r", 0, bin_fc, 0, -1, BIN_R, "nrl", NULL),
@@ -238,10 +238,8 @@
 int
 execbuiltin(LinkList args, Builtin bn)
 {
-    LinkNode n;
-    char *arg, *pp, *name, *optstr;
-    char *oxarg, *xarg = NULL;
-    int flags, sense, argc = 0, execop, xtr = isset(XTRACE), lxarg = 0;
+    char *pp, *name, *optstr;
+    int flags, sense, argc, execop, xtr = isset(XTRACE);
     struct options ops;
 
     /* initialise options structure */
@@ -252,8 +250,6 @@
     /* initialize some local variables */
     name = (char *) ugetnode(args);
 
-    arg = (char *) ugetnode(args);
-
     if (!bn->handlerfunc) {
 	zwarnnam(name, "autoload failed", NULL, 0);
 	deletebuiltin(bn->nam);
@@ -263,141 +259,150 @@
     flags = bn->flags;
     optstr = bn->optstr;
 
-    /* Sort out the options. */
-    if ((flags & BINF_ECHOPTS) && isset(BSDECHO))
-	ops.ind['E'] = 1;
-    if (optstr)
-	/* while arguments look like options ... */
-	while (arg &&
-	       ((sense = (*arg == '-')) ||
-		((flags & BINF_PLUSOPTS) && *arg == '+')) &&
-	       ((flags & BINF_PLUSOPTS) || !atoi(arg))) {
-	    /* unrecognised options to echo etc. are not really options */
-	    if (flags & BINF_ECHOPTS) {
-		char *p = arg;
-		while (*++p && strchr(optstr, (int) *p));
-		if (*p)
+    /* Set up the argument list. */
+    /* count the arguments */
+    argc = countlinknodes(args);
+
+    {
+	/*
+	 * Keep all arguments, including options, in an array.
+	 * We don't actually need the option part of the argument
+	 * after option processing, but it makes XTRACE output
+	 * much simpler.
+	 */
+	VARARR(char *, argarr, argc + 1);
+	char **argv;
+
+	/*
+	 * Get the actual arguments, into argv.  Remember argarr
+	 * may be an array declaration, depending on the compiler.
+	 */
+	argv = argarr;
+	while ((*argv++ = (char *)ugetnode(args)));
+	argv = argarr;
+
+	/* Sort out the options. */
+	if (optstr) {
+	    char *arg = *argv;
+	    /* while arguments look like options ... */
+	    while (arg &&
+		   /* Must begin with - or maybe + */
+		   ((sense = (*arg == '-')) ||
+		    ((flags & BINF_PLUSOPTS) && *arg == '+'))) {
+		/* Digits aren't arguments unless the command says they are. */
+		if (!(flags & BINF_KEEPNUM) && idigit(arg[1]))
+		    break;
+		/* For cd and friends, a single dash is not an option. */
+		if ((flags & BINF_SKIPDASH) && !arg[1])
+		    break;
+		if ((flags & BINF_DASHDASHVALID) && !strcmp(arg, "--")) {
+		    /*
+		     * Need to skip this before checking whether this is
+		     * really an option.
+		     */
+		    argv++;
 		    break;
-	    }
-	    /* save the options in xarg, for execution tracing */
-	    if (xtr) {
-		if (xarg) {
-		    int l = strlen(arg) + lxarg + 1;
-
-		    oxarg = zhalloc(l + 1);
-		    strcpy(oxarg, xarg);
-		    oxarg[lxarg] = ' ';
-		    strcpy(oxarg + lxarg + 1, arg);
-		    xarg = oxarg;
-		    lxarg = l + 1;
-		} else {
-		    xarg = dupstring(arg);
-		    lxarg = strlen(xarg);
 		}
-	    }
-	    /* handle -- or - (ops.ind['-']), and +
-	     * (ops.ind['-'] and ops.ind['+']) */
-	    if (arg[1] == '-')
-		arg++;
-	    if (!arg[1]) {
-		ops.ind['-'] = 1;
-		if (!sense)
-		    ops.ind['+'] = 1;
-	    }
-	    /* save options in ops, as long as they are in bn->optstr */
-	    execop = -1;
-	    while (*++arg) {
-		char *optptr;
-		if ((optptr = strchr(optstr, execop = (int)*arg))) {
-		    ops.ind[(int)*arg] = (sense) ? 1 : 2;
-		    if (optptr[1] == ':') {
-			char *argptr = NULL;
-			if (optptr[2] == ':') {
-			    if (arg[1])
-				argptr = arg+1;
-			    /* Optional argument in same word*/
-			} else if (optptr[2] == '%') {
-			    /* Optional numeric argument in same
-			     * or next word. */
-			    if (arg[1] && idigit(arg[1]))
-				argptr = arg+1;
-			    else if (firstnode(args) &&
-				     idigit(*(char *)peekfirst(args)))
-				argptr = arg = (char *)ugetnode(args);
-			} else {
-			    /* Mandatory argument */
-			    if (arg[1])
-				argptr = arg+1;
-			    else if ((arg = (char *)ugetnode(args)))
-				argptr = arg;
-			    else {
-				zwarnnam(name, "argument expected: -%c", NULL,
-					 execop);
-				return 1;
+		/* unrecognised options to echo etc. are not really options */
+		if (flags & BINF_SKIPINVALID) {
+		    char *p = arg;
+		    if (optstr)
+			while (*++p && strchr(optstr, (int) *p));
+		    else
+			p++;
+		    if (*p)
+			break;
+		}
+		/* handle -- or - (ops.ind['-']), and +
+		 * (ops.ind['-'] and ops.ind['+']) */
+		if (arg[1] == '-')
+		    arg++;
+		if (!arg[1]) {
+		    ops.ind['-'] = 1;
+		    if (!sense)
+			ops.ind['+'] = 1;
+		}
+		/* save options in ops, as long as they are in bn->optstr */
+		while (*++arg) {
+		    char *optptr;
+		    if ((optptr = strchr(optstr, execop = (int)*arg))) {
+			ops.ind[(int)*arg] = (sense) ? 1 : 2;
+			if (optptr[1] == ':') {
+			    char *argptr = NULL;
+			    if (optptr[2] == ':') {
+				if (arg[1])
+				    argptr = arg+1;
+				/* Optional argument in same word*/
+			    } else if (optptr[2] == '%') {
+				/* Optional numeric argument in same
+				 * or next word. */
+				if (arg[1] && idigit(arg[1]))
+				    argptr = arg+1;
+				else if (argv[1] && idigit(*argv[1]))
+				    argptr = arg = *++argv;
+			    } else {
+				/* Mandatory argument */
+				if (arg[1])
+				    argptr = arg+1;
+				else if ((arg = *++argv))
+				    argptr = arg;
+				else {
+				    zwarnnam(name, "argument expected: -%c",
+					     NULL, execop);
+				    return 1;
+				}
 			    }
-			}
-			if (argptr) {
-			    if (new_optarg(&ops)) {
-				zwarnnam(name, 
-					 "too many option arguments", NULL, 0);
-				return 1;
+			    if (argptr) {
+				if (new_optarg(&ops)) {
+				    zwarnnam(name, 
+					     "too many option arguments",
+					     NULL, 0);
+				    return 1;
+				}
+				ops.ind[execop] |= ops.argscount << 2;
+				ops.args[ops.argscount-1] = argptr;
+				while (arg[1])
+				    arg++;
 			    }
-			    ops.ind[execop] |= ops.argscount << 2;
-			    ops.args[ops.argscount-1] = argptr;
-			    while (arg[1])
-				arg++;
 			}
-		    }
-		} else
+		    } else
+			break;
+		}
+		/* The above loop may have exited on an invalid option.  (We  *
+		 * assume that any option requiring metafication is invalid.) */
+		if (*arg) {
+		    if(*arg == Meta)
+			*++arg ^= 32;
+		    zwarn("bad option: -%c", NULL, *arg);
+		    return 1;
+		}
+		arg = *++argv;
+		/* for the "print" builtin, the options after -R are treated as
+		   options to "echo" */
+		if ((flags & BINF_PRINTOPTS) && ops.ind['R'] &&
+		    !ops.ind['f']) {
+		    optstr = "ne";
+		    flags |= BINF_SKIPINVALID;
+		}
+		/* the option -- indicates the end of the options */
+		if (ops.ind['-'])
 		    break;
 	    }
-	    /* The above loop may have exited on an invalid option.  (We  *
-	     * assume that any option requiring metafication is invalid.) */
-	    if (*arg) {
-		if(*arg == Meta)
-		    *++arg ^= 32;
-		zwarn("bad option: -%c", NULL, *arg);
-		return 1;
-	    }
-	    arg = (char *) ugetnode(args);
-	    /* for the "print" builtin, the options after -R are treated as
-	       options to "echo" */
-	    if ((flags & BINF_PRINTOPTS) && ops.ind['R'] && !ops.ind['f']) {
-		optstr = "ne";
-		flags |= BINF_ECHOPTS;
-	    }
-	    /* the option -- indicates the end of the options */
-	    if (ops.ind['-'])
-		break;
 	}
-    /* handle built-in options, for overloaded handler functions */
-    if ((pp = bn->defopts)) {
-	while (*pp) {
-	    /* only if not already set */
-	    if (!ops.ind[(int)*pp])
-		ops.ind[(int)*pp] = 1;
-	    pp++;
+
+	/* handle built-in options, for overloaded handler functions */
+	if ((pp = bn->defopts)) {
+	    while (*pp) {
+		/* only if not already set */
+		if (!ops.ind[(int)*pp])
+		    ops.ind[(int)*pp] = 1;
+		pp++;
+	    }
 	}
-    }
 
-    /* Set up the argument list. */
-    if (arg) {
-	/* count the arguments */
-	argc = 1;
-	n = firstnode(args);
-	while (n)
-	    argc++, incnode(n);
-    }
-    {
-	VARARR(char *, argarr, (argc + 1));
-	char **argv, **oargv;
+	/* Fix the argument count by subtracting option arguments */
+	argc -= argv - argarr;
 
-	/* Get the actual arguments, into argv.  Oargv saves the *
-	 * beginning of the array for later reference.           */
-	oargv = argv = argarr;
-	if ((*argv++ = arg))
-	    while ((*argv++ = (char *)ugetnode(args)));
-	argv = oargv;
 	if (errflag) {
 	    errflag = 0;
 	    return 1;
@@ -412,15 +417,13 @@
 
 	/* display execution trace information, if required */
 	if (xtr) {
+	    /* Use full argument list including options for trace output */
+	    char **fullargv = argarr;
 	    printprompt4();
 	    fprintf(xtrerr, "%s", name);
-	    if (xarg) {
-	        fputc(' ', xtrerr);
-	        quotedzputs(xarg, xtrerr);
-	    }
-	    while (*oargv) {
+	    while (*fullargv) {
 	        fputc(' ', xtrerr);
-	        quotedzputs(*oargv++, xtrerr);
+	        quotedzputs(*fullargv++, xtrerr);
 	    }
 	    fputc('\n', xtrerr);
 	    fflush(xtrerr);
@@ -762,27 +765,11 @@
     }
     doprintdir = (doprintdir == -1);
 
-    for (; *argv && **argv == '-'; argv++) {
-	char *s = *argv + 1;
-
-	do {
-	    switch (*s) {
-	    case 's':
-	    case 'P':
-	    case 'L':
-		break;
-	    default:
-		goto brk;
-	    }
-	} while (*++s);
-	for (s = *argv; *++s; ops->ind[STOUC(*s)] = 1);
-    }
- brk:
     chasinglinks = OPT_ISSET(ops,'P') || 
 	(isset(CHASELINKS) && !OPT_ISSET(ops,'L'));
     queue_signals();
     zpushnode(dirstack, ztrdup(pwd));
-    if (!(dir = cd_get_dest(nam, argv, ops, func))) {
+    if (!(dir = cd_get_dest(nam, argv, OPT_ISSET(ops,'s'), func))) {
 	zsfree(getlinknode(dirstack));
 	unqueue_signals();
 	return 1;
@@ -812,7 +799,7 @@
 
 /**/
 static LinkNode
-cd_get_dest(char *nam, char **argv, Options ops, int func)
+cd_get_dest(char *nam, char **argv, int hard, int func)
 {
     LinkNode dir = NULL;
     LinkNode target;
@@ -882,7 +869,7 @@
     if (!dir) {
 	dir = firstnode(dirstack);
     }
-    if (!(dest = cd_do_chdir(nam, getdata(dir), OPT_ISSET(ops,'s')))) {
+    if (!(dest = cd_do_chdir(nam, getdata(dir), hard))) {
 	if (!target)
 	    zsfree(getlinknode(dirstack));
 	if (func == BIN_POPD)
@@ -3026,6 +3013,8 @@
     
     if (func == BIN_PRINTF)
 	fmt = *args++;
+    else if (func == BIN_ECHO && isset(BSDECHO))
+	ops->ind['E'] = 1;
     else if (OPT_HASARG(ops,'f'))
 	fmt = OPT_ARG(ops,'f');
     if (fmt)
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.41
diff -u -r1.41 zsh.h
--- Src/zsh.h	27 Aug 2002 21:10:34 -0000	1.41
+++ Src/zsh.h	2 Sep 2002 13:16:31 -0000
@@ -1002,15 +1002,19 @@
 #define BINF_PLUSOPTS		(1<<1)	/* +xyz legal */
 #define BINF_PRINTOPTS		(1<<2)
 #define BINF_ADDED		(1<<3)	/* is in the builtins hash table */
-#define BINF_ECHOPTS		(1<<4)
-#define BINF_MAGICEQUALS	(1<<5)  /* needs automatic MAGIC_EQUAL_SUBST substitution */
-#define BINF_PREFIX		(1<<6)
-#define BINF_DASH		(1<<7)
-#define BINF_BUILTIN		(1<<8)
-#define BINF_COMMAND		(1<<9)
-#define BINF_EXEC		(1<<10)
-#define BINF_NOGLOB		(1<<11)
-#define BINF_PSPECIAL		(1<<12)
+#define BINF_MAGICEQUALS	(1<<4)  /* needs automatic MAGIC_EQUAL_SUBST substitution */
+#define BINF_PREFIX		(1<<5)
+#define BINF_DASH		(1<<6)
+#define BINF_BUILTIN		(1<<7)
+#define BINF_COMMAND		(1<<8)
+#define BINF_EXEC		(1<<9)
+#define BINF_NOGLOB		(1<<10)
+#define BINF_PSPECIAL		(1<<11)
+/* Builtin option handling */
+#define BINF_SKIPINVALID	(1<<12)	/* Treat invalid option as argument */
+#define BINF_KEEPNUM		(1<<13) /* `[-+]NUM' can be an option */
+#define BINF_SKIPDASH		(1<<14) /* Treat `-' as argument (maybe `+') */
+#define BINF_DASHDASHVALID	(1<<15) /* Handle `--' evenf if SKIPINVALD */
 
 struct module {
     char *nam;
-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************



Messages sorted by: Reverse Date, Date, Thread, Author