Re: PATCH: zsh/files adjustments

[Peter Stephenson <pws@xxxxxxx>]

On Thu, 8 May 2008 10:05:35 +0100
Peter Stephenson <pws@xxxxxxx> wrote:
> On Wed, 7 May 2008 23:08:13 -0700
> Phil Pennock <zsh-workers+phil.pennock@xxxxxxxxxxxx> wrote:
> > Is there any thought to being able to name groups of features, perhaps
> > with a g: prefix, so that it becomes possible to do:
> >   zmodload -F zsh/files +g:zf_commands
> >   zmodload -F zsh/files -g:bare_commands
> > ?  Is this desirable?  I'd view the semantics as batch-enable/disable,
> > so that later items on the command-line override, even if more general.
> 
> Something like this is probably desirable, but we might get away with an
> option to handle features based on pattern.

...which is relatively straightforward to add and fully consistent with
lots of other builtins, so is probably worth doing anyway.  Adding struct
feature_enables meant the majority of cases that just pass a NULL
into the module system to specify feature lists can continue to do so.

zmodload -mF zsh/files 'b:zf_*'
setopt extendedglob
zmodload -mf zsh/files 'b:^zf_*'

Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.104
diff -u -r1.104 builtins.yo
--- Doc/Zsh/builtins.yo	28 Mar 2008 09:59:08 -0000	1.104
+++ Doc/Zsh/builtins.yo	8 May 2008 11:58:03 -0000
@@ -1901,7 +1901,7 @@
 cindex(modules, loading)
 cindex(loading modules)
 xitem(tt(zmodload) [ tt(-dL) ] [ ... ])
-xitem(tt(zmodload -F) [ tt(-lLe) tt(-P) tt(param) ] var(module) [tt(PLUS()-)]var(feature...))
+xitem(tt(zmodload -F) [ tt(-lLme) tt(-P) tt(param) ] var(module) [tt(PLUS()-)]var(feature...))
 xitem(tt(zmodload -e) [ tt(-A) ] [ ... ])
 xitem(tt(zmodload) [ tt(-a) [ tt(-bcpf) [ tt(-I) ] ] ] [ tt(-iL) ] ...)
 xitem(tt(zmodload) tt(-u) [ tt(-abcdpf) [ tt(-I) ] ] [ tt(-iL) ] ...)
@@ -1952,7 +1952,7 @@
 will not be loaded if its boot function fails.  Similarly a module
 can only be unloaded if its cleanup function runs successfully.
 )
-item(tt(zmodload -F) [ tt(-alLe) tt(-P) tt(param) ] var(module) [tt(PLUS()-)]var(feature...))(
+item(tt(zmodload -F) [ tt(-almLe) tt(-P) tt(param) ] var(module) [tt(PLUS()-)]var(feature...))(
 tt(zmodload -F) allows more selective control over the features provided
 by modules.  With no options apart from tt(-F), the module named
 var(module) is loaded, if it was not already loaded, and the list of
@@ -2006,6 +2006,12 @@
 see if is provided and in the given state.  If the tests on all features
 in the list succeed, status 0 is returned, else status 1.
 
+With tt(-m), each entry in the given list of features is taken
+as a pattern to be matched against the list of features provided
+by the module.  An initial tt(PLUS()) or tt(-) must be given explicitly.
+This may not be combined with the tt(-a) option as autoloads must
+be specified explicitly.
+
 With tt(-a), the given list of features is marked for autoload from
 the specified module, which may not yet be loaded.  An optional tt(PLUS())
 may appear before the feature name.  If the feature is prefixed with
@@ -2015,7 +2021,7 @@
 enabled.  Autoload requests are preserved if the module is
 subsequently unloaded until an explicit `tt(zmodload -Fa) var(module)
 tt(-)var(feature)' is issued.  It is not an error to request an autoload
-for a feature of a module that is already loaded.  
+for a feature of a module that is already loaded.
 
 When the module is loaded each autoload is checked against the features
 actually provided by the module; if the feature is not provided the
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.191
diff -u -r1.191 builtin.c
--- Src/builtin.c	22 Apr 2008 15:08:12 -0000	1.191
+++ Src/builtin.c	8 May 2008 11:58:09 -0000
@@ -131,7 +131,7 @@
     BUILTIN("whence", 0, bin_whence, 0, -1, 0, "acmpvfsw", NULL),
     BUILTIN("where", 0, bin_whence, 0, -1, 0, "pmsw", "ca"),
     BUILTIN("which", 0, bin_whence, 0, -1, 0, "ampsw", "c"),
-    BUILTIN("zmodload", 0, bin_zmodload, 0, -1, 0, "AFRILP:abcfdilpue", NULL),
+    BUILTIN("zmodload", 0, bin_zmodload, 0, -1, 0, "AFRILP:abcfdilmpue", NULL),
     BUILTIN("zcompile", 0, bin_zcompile, 0, -1, 0, "tUMRcmzka", NULL),
 };
 
Index: Src/module.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/module.c,v
retrieving revision 1.36
diff -u -r1.36 module.c
--- Src/module.c	31 Jul 2007 14:35:52 -0000	1.36
+++ Src/module.c	8 May 2008 11:58:10 -0000
@@ -1967,7 +1967,7 @@
 
 /**/
 static int
-do_module_features(Module m, char **enablesstr, int flags)
+do_module_features(Module m, Feature_enables enablesarr, int flags)
 {
     char **features;
     int ret = 0;
@@ -2019,19 +2019,26 @@
 		    (void)autofeatures(NULL, m->node.nam, arg, 0,
 				       FEAT_IGNORE|FEAT_REMOVE);
 		    /*
-		     * don't want to try to enable *that*... 
+		     * don't want to try to enable *that*...
 		     * expunge it from the enable string.
 		     */
-		    if (enablesstr) {
-			for (ptr = enablesstr; *ptr; ptr++) {
-			    if (!strcmp(al, *ptr)) {
+		    if (enablesarr) {
+			Feature_enables fep;
+			for (fep = enablesarr; fep->str; fep++) {
+			    char *str = fep->str;
+			    if (*str == '+' || *str == '-')
+				str++;
+			    if (fep->pat ? pattry(fep->pat, al) :
+				!strcmp(al, str)) {
 				/* can't enable it after all, so return 1 */
 				ret = 1;
-				while (*ptr) {
-				    *ptr = ptr[1];
-				    ptr++;
+				while (fep->str) {
+				    fep->str = fep[1].str;
+				    fep->pat = fep[1].pat;
+				    fep++;
 				}
-				break;
+				if (!fep->pat)
+				    break;
 			    }
 			}
 		    }
@@ -2039,11 +2046,11 @@
 	    }
 	}
 
-	if (enablesstr) {
-	    char **ep;
-	    for (ep = enablesstr; *ep; ep++) {
-		char **fp, *esp = *ep;
-		int on = 1;
+	if (enablesarr) {
+	    Feature_enables fep;
+	    for (fep = enablesarr; fep->str; fep++) {
+		char **fp, *esp = fep->str;
+		int on = 1, found = 0;
 		if (*esp == '+')
 		    esp++;
 		else if (*esp == '-') {
@@ -2051,13 +2058,17 @@
 		    esp++;
 		}
 		for (fp = features; *fp; fp++)
-		    if (!strcmp(*fp, esp)) {
+		    if (fep->pat ? pattry(fep->pat, *fp) : !strcmp(*fp, esp)) {
 			enables[fp - features] = on;
-			break;
+			found++;
+			if (!fep->pat)
+			    break;
 		    }
-		if (!*fp) {
+		if (!found) {
 		    if (!(flags & FEAT_IGNORE))
-			zwarn("module `%s' has no such feature: `%s'",
+			zwarn(fep->pat ?
+			      "module `%s' has no feature matching: `%s'" :
+			      "module `%s' has no such feature: `%s'",
 			      m->node.nam, esp);
 		    return 1;
 		}
@@ -2075,7 +2086,7 @@
 
 	if (enables_module(m, &enables))
 	    return 2;
-    } else if (enablesstr) {
+    } else if (enablesarr) {
 	if (!(flags & FEAT_IGNORE))
 	    zwarn("module `%s' does not support features", m->node.nam);
 	return 1;
@@ -2097,9 +2108,9 @@
 
 /**/
 static int
-do_boot_module(Module m, char **enablesstr, int silent)
+do_boot_module(Module m, Feature_enables enablesarr, int silent)
 {
-    int ret = do_module_features(m, enablesstr,
+    int ret = do_module_features(m, enablesarr,
 				 silent ? FEAT_IGNORE|FEAT_CHECKAUTO :
 				 FEAT_CHECKAUTO);
 
@@ -2159,7 +2170,7 @@
 
 /**/
 mod_export int
-load_module(char const *name, char **enablesstr, int silent)
+load_module(char const *name, Feature_enables enablesarr, int silent)
 {
     Module m;
     void *handle = NULL;
@@ -2194,7 +2205,7 @@
 	modulestab->addnode(modulestab, ztrdup(name), m);
 
 	if ((set = setup_module(m)) ||
-	    (bootret = do_boot_module(m, enablesstr, silent)) == 1) {
+	    (bootret = do_boot_module(m, enablesarr, silent)) == 1) {
 	    if (!set)
 		do_cleanup_module(m);
 	    finish_module(m);
@@ -2263,7 +2274,7 @@
 	m->node.flags |= MOD_INIT_S;
     }
     m->node.flags |= MOD_SETUP;
-    if ((bootret = do_boot_module(m, enablesstr, silent)) == 1) {
+    if ((bootret = do_boot_module(m, enablesarr, silent)) == 1) {
 	do_cleanup_module(m);
 	finish_module(m);
 	if (m->node.flags & MOD_LINKED)
@@ -2301,7 +2312,7 @@
 
 /**/
 mod_export int
-require_module(const char *module, char **features)
+require_module(const char *module, Feature_enables features)
 {
     Module m = NULL;
     int ret = 0;
@@ -2962,7 +2973,10 @@
 static int
 bin_zmodload_features(const char *nam, char **args, Options ops)
 {
+    int iarg;
     char *modname = *args;
+    Patprog *patprogs;
+    Feature_enables features, fep;
 
     if (modname)
 	args++;
@@ -2986,6 +3000,23 @@
 	return 1;
     }
 
+    if (OPT_ISSET(ops,'m')) {
+	char **argp;
+	Patprog *patprogp;
+
+	/* not NULL terminated */
+	patprogp = patprogs =
+	    (Patprog *)zhalloc(arrlen(args)*sizeof(Patprog));
+	for (argp = args; *argp; argp++, patprogp++) {
+	    char *arg = *argp;
+	    if (*arg == '+' || *arg == '-')
+		arg++;
+	    tokenize(arg);
+	    *patprogp = patcompile(arg, 0, 0);
+	}
+    } else
+	patprogs = NULL;
+
     if (OPT_ISSET(ops,'l') || OPT_ISSET(ops,'L') || OPT_ISSET(ops,'e')) {
 	/*
 	 * With option 'l', list all features one per line with + or -.
@@ -3063,9 +3094,9 @@
 		     m->node.nam);
 	    return 1;
 	}
-	for (arrp = args; *arrp; arrp++) {
+	for (arrp = args, iarg = 0; *arrp; arrp++, iarg++) {
 	    char *arg = *arrp;
-	    int on;
+	    int on, found = 0;
 	    if (*arg == '-') {
 		on = 0;
 		arg++;
@@ -3075,17 +3106,22 @@
 	    } else
 		on = -1;
 	    for (fp = features, ep = enables; *fp; fp++, ep++) {
-		if (!strcmp(arg, *fp)) {
+		if (patprogs ? pattry(patprogs[iarg], *fp) :
+		    !strcmp(arg, *fp)) {
 		    /* for -e, check given state, if any */
 		    if (OPT_ISSET(ops,'e') && on != -1 &&
 			on != (*ep & 1))
 			return 1;
-		    break;
+		    found++;
+		    if (!patprogs)
+			break;
 		}
 	    }
-	    if (!*fp) {
+	    if (!found) {
 		if (!OPT_ISSET(ops,'e'))
-		    zwarnnam(nam, "module `%s' has no such feature: `%s'",
+		    zwarnnam(nam, patprogs ?
+			     "module `%s' has no feature matching: `%s'" :
+			     "module `%s' has no such feature: `%s'",
 			     *arrp);
 		return 1;
 	    }
@@ -3100,12 +3136,13 @@
 		    continue;
 		if (*args) {
 		    char **argp;
-		    for (argp = args; *argp; argp++) {
+		    for (argp = args, iarg = 0; *argp; argp++, iarg++) {
 			char *arg = *argp;
 			/* ignore +/- for consistency */
 			if (*arg == '+' || *arg == '-')
 			    arg++;
-			if (!strcmp(*fp, arg))
+			if (patprogs ? pattry(patprogs[iarg], *fp) :
+			    !strcmp(*fp, arg))
 			    break;
 		    }
 		    if (!*argp)
@@ -3121,11 +3158,12 @@
 	    int term;
 	    if (*args) {
 		char **argp;
-		for (argp = args; *argp; argp++) {
+		for (argp = args, iarg = 0; *argp; argp++, iarg++) {
 		    char *arg = *argp;
 		    if (*arg == '+' || *arg == '-')
 			arg++;
-		    if (!strcmp(*fp, *argp))
+		    if (patprogs ? pattry(patprogs[iarg], *fp) :
+			!strcmp(*fp, *argp))
 			break;
 		}
 		if (!*argp)
@@ -3161,6 +3199,10 @@
 	zwarnnam(nam, "-P can only be used with -l or -L");
 	return 1;
     } else if (OPT_ISSET(ops,'a')) {
+	if (OPT_ISSET(ops,'m')) {
+	    zwarnnam(nam, "-m cannot be used with -a");
+	    return 1;
+	}
 	/*
 	 * With zmodload -aF, we always use the effect of -i.
 	 * The thinking is that marking a feature for
@@ -3175,7 +3217,18 @@
 	return autofeatures(nam, modname, args, 0, FEAT_IGNORE);
     }
 
-    return require_module(modname, args);
+    fep = features =
+	(Feature_enables)zhalloc((arrlen(args)+1)*sizeof(*fep));
+
+    while (*args) {
+	fep->str = *args++;
+	fep->pat = patprogs ? *patprogs++ : NULL;
+	fep++;
+    }
+    fep->str = NULL;
+    fep->pat = NULL;
+
+    return require_module(modname, features);
 }
 
 
@@ -3330,14 +3383,17 @@
 mod_export int
 ensurefeature(const char *modname, const char *prefix, const char *feature)
 {
-    char *f, *features[2];
+    char *f;
+    struct feature_enables features[2];
 
     if (!feature)
 	return require_module(modname, NULL);
     f = dyncat(prefix, feature);
 
-    features[0] = f;
-    features[1] = NULL;
+    features[0].str = f;
+    features[0].pat = NULL;
+    features[1].str = NULL;
+    features[1].pat = NULL;
     return require_module(modname, features);
 }
 
@@ -3435,7 +3491,7 @@
 	    zwarnnam(cmdnam, "%s: `/' is illegal in a %s", fnam, typnam);
 	    ret = 1;
 	    continue;
-	} 
+	}
 
 	if (!module) {
 	    /*
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.130
diff -u -r1.130 zsh.h
--- Src/zsh.h	6 May 2008 09:23:08 -0000	1.130
+++ Src/zsh.h	8 May 2008 11:58:12 -0000
@@ -382,6 +382,7 @@
 typedef struct complist  *Complist;
 typedef struct conddef   *Conddef;
 typedef struct features  *Features;
+typedef struct feature_enables  *Feature_enables;
 typedef struct funcstack *Funcstack;
 typedef struct funcwrap  *FuncWrap;
 typedef struct hashnode  *HashNode;
@@ -1247,6 +1248,16 @@
     int n_abstract;
 };
 
+/*
+ * Structure describing enables for one feature.
+ */
+struct feature_enables {
+    /* String feature to enable (N.B. no leading +/- allowed) */
+    char *str;
+    /* Optional compiled pattern for str sans +/-, NULL for string match */
+    Patprog pat;
+};
+
 /* C-function hooks */
 
 typedef int (*Hookfn) _((Hookdef, void *));


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