diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c index ef9148d7b..43265f676 100644 --- a/Src/Modules/parameter.c +++ b/Src/Modules/parameter.c @@ -1114,7 +1114,8 @@ scanpmmodules(UNUSED(HashTable ht), ScanFunc func, int flags) } for (i = 0; i < realparamtab->hsize; i++) for (hn = realparamtab->nodes[i]; hn; hn = hn->next) { - if ((((Param) hn)->node.flags & PM_AUTOLOAD) && + int flags = ((Param) hn)->node.flags; + if ((flags & PM_AUTOLOAD) && !(flags & PM_UNSET) && !linknodebystring(done, ((Param) hn)->u.str)) { pm.node.nam = ((Param) hn)->u.str; addlinknode(done, pm.node.nam); diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 8c0534708..6c88a9157 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -2239,6 +2239,8 @@ setup_(UNUSED(Module m)) bpaste[0] = ztrdup("\033[?2004h"); bpaste[1] = ztrdup("\033[?2004l"); /* Intended to be global, no WARNCREATEGLOBAL check. */ + /* ### TODO: Don't set it if it's already set */ + /* ### TODO: Make it a module parameter to let it be preemptively unset in zshrc */ assignaparam("zle_bracketed_paste", bpaste, 0); return 0; diff --git a/Src/module.c b/Src/module.c index f41b82f25..7ac93ee4b 100644 --- a/Src/module.c +++ b/Src/module.c @@ -1018,6 +1018,7 @@ runhookdef(Hookdef h, void *d) * requires that either there's no parameter already present, * or it's a global parameter marked for autoloading. * + * Return status is zero on success, non-zero on failure. * The special status 2 is to indicate it didn't work but * -i was in use so we didn't print a warning. */ @@ -1030,7 +1031,8 @@ checkaddparam(const char *nam, int opt_i) if (!(pm = (Param) gethashnode2(paramtab, nam))) return 0; - if (pm->level || !(pm->node.flags & PM_AUTOLOAD)) { + if (pm->level + || (pm->node.flags & (PM_AUTOLOAD|PM_UNSET)) != PM_AUTOLOAD) { /* * -i suppresses "it's already that way" warnings, * but not "this can't possibly work" warnings, so we print @@ -1056,12 +1058,20 @@ checkaddparam(const char *nam, int opt_i) /* This adds the given parameter definition. The return value is zero on * * success and 1 on failure. */ -/**/ -int +static int addparamdef(Paramdef d) { Param pm; + /* If there's already a "tombstone", honour it and don't add the parameter. */ + /* ### TODO: verify that a tombstone can be removed by the user if desired */ + { + Param pm2 = (Param) gethashnode2(paramtab, d->name); + if (pm2 && pm2->node.flags & PM_AUTOLOAD && pm2->node.flags & PM_UNSET) { + return 0; + } + } + if (checkaddparam(d->name, 0)) return 1; @@ -1199,6 +1209,16 @@ add_autoparam(const char *module, const char *pnam, int flags) Param pm; int ret; + /* If there's already a "tombstone" parameter, we simply need to + * revive it. */ + { + Param pm2 = (Param) gethashnode2(paramtab, pnam); + if (pm2 && pm2->node.flags & PM_AUTOLOAD && pm2->node.flags & PM_UNSET) { + pm2->node.flags &= ~PM_UNSET; + return 0; + } + } + queue_signals(); if ((ret = checkaddparam(pnam, (flags & FEAT_IGNORE)))) { unqueue_signals(); diff --git a/Src/params.c b/Src/params.c index 863b32600..9d4548720 100644 --- a/Src/params.c +++ b/Src/params.c @@ -527,7 +527,7 @@ getparamnode(HashTable ht, const char *nam) (void)ensurefeature(mn, "p:", (pm->node.flags & PM_AUTOALL) ? NULL : nam); hn = gethashnode2(ht, nam); - if (!hn) { + if (!hn || (pm->node.flags & PM_UNSET)) { /* * This used to be a warning, but surely if we allow * stuff to go ahead with the autoload stub with @@ -3604,6 +3604,7 @@ unsetparam_pm(Param pm, int altflag, int exp) { Param oldpm, altpm; char *altremove; + int node_removed = 0; /* boolean flag */ if ((pm->node.flags & PM_READONLY) && pm->level <= locallevel) { zerr("read-only variable: %s", pm->node.nam); @@ -3670,8 +3671,21 @@ unsetparam_pm(Param pm, int altflag, int exp) (pm->node.flags & (PM_SPECIAL|PM_REMOVABLE)) == PM_SPECIAL) return 0; - /* remove parameter node from table */ - paramtab->removenode(paramtab, pm->node.nam); + /* + * Remove parameter node from table. + * + * For autoloaded parameters, we leave the Param behind with + * the PM_UNSET flag on, as a "tombstone" so zmodload won't + * try to create this parameter later. Cf. add_autoparam() + * and getparamnode(). + */ + if (pm->node.flags & PM_AUTOLOAD) + /* ### TODO: audit all uses of PM_AUTOLOAD to see if they need to handle tombstones */ + pm->node.flags |= PM_UNSET; + else { + paramtab->removenode(paramtab, pm->node.nam); + node_removed = 1; + } if (pm->old) { oldpm = pm->old; @@ -3692,7 +3706,8 @@ unsetparam_pm(Param pm, int altflag, int exp) } } - paramtab->freenode(&pm->node); /* free parameter node */ + if (node_removed) + paramtab->freenode(&pm->node); /* free parameter node */ return 0; } diff --git a/Test/V01zmodload.ztst b/Test/V01zmodload.ztst index 0a7fbb651..077a252dc 100644 --- a/Test/V01zmodload.ztst +++ b/Test/V01zmodload.ztst @@ -353,20 +353,19 @@ if zmodload -e zsh/parameter; then zmodload -u zsh/parameter; fi unset options zmodload zsh/parameter - echo \$+options + echo \$+options +\$options+ " --f:can unset a non-readonly autoloadable parameter before loading the module ->0 -# Currently prints '1'. +0:can unset a non-readonly autoloadable parameter before loading the module +>0 ++ $ZTST_testdir/../Src/zsh -fc " MODULE_PATH=${(q)MODULE_PATH} zmodload zsh/parameter unset options - echo \$+options + echo \$+options +\$options+ " 0:can unset a non-readonly autoloadable parameter after loading the module ->0 +>0 ++ $ZTST_testdir/../Src/zsh -fc " MODULE_PATH=${(q)MODULE_PATH} @@ -375,7 +374,10 @@ " -f:can't unset a readonly autoloadable parameter before loading the module *?zsh:?: read-only variable: builtins -# Currently, the 'unset' succeeds. +# Currently, the 'unset' succeeds. For it to fail, the code would need to +# know that $builtins is read-only. However, that information only becomes +# available after features_() is called, which happens after loading the +# module. $ZTST_testdir/../Src/zsh -fc " MODULE_PATH=${(q)MODULE_PATH}