Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: Unset “zle_bracketed_paste” .zshrc
Peter Stephenson wrote on Thu, 06 Feb 2020 14:09 +0000:
> On Thu, 2020-02-06 at 14:32 +0100, Mikael Magnusson wrote:
> > On 2/6/20, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > Well, it's an option. It would result in the following interface:
> > > to prevent $zle_bracketed_paste from being defined one will have to run
> > > «unset zle_bracketed_paste» if zsh/zle has been loaded, or «zmodload -Fa
> > > zsh/zle -p:zle_bracketed_paste» otherwise.
> > I would assume that if someone knew to run that obscure zmodload
> > command, they would know they could just do this instead,
> > zmodload zsh/zle
> > unset zle_bracketed_paste
>
> No, this is entirely about autoload behaviour. "zmodload -Fa" provides
> selective autoload out of the box, which in principle fixes the issue
> with no fundamentally new features, just appropriate module exports.
>
> However, the main reason for doing this with zmodload would be to
> provide consistency with different types of module feature, and there's
> no real call for that as people are much more used to the parameter
> interface. So in practice adding behaviour to "unset" is probably
> easier for everyone.
>
I've got a WIP patch for this that I'd like to share. It fixes the
xfail ('f'-status) test; however, I'm not sure everything in it's
correct, and there are a few outstanding todo's. It's nowhere near
being ready to be committed; I'm only posting it now for 'release early
and often' reasons.
It's attached.
Cheers,
Daniel
> It would be neater to be more consistent about the way module features
> are provided, just on the basis that if it's in a module it should use
> the module interface, else why are we pretending it's a module at all
> rather than building it into the shell? But that can go way down a very
> long list.
>
> pws
>
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}
Messages sorted by:
Reverse Date,
Date,
Thread,
Author