From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE,RDNS_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 Received: (qmail 25838 invoked from network); 14 Mar 2020 03:29:45 -0000 Received-SPF: pass (primenet.com.au: domain of zsh.org designates 203.24.36.2 as permitted sender) receiver=inbox.vuxu.org; client-ip=203.24.36.2 envelope-from= Received: from unknown (HELO primenet.com.au) (203.24.36.2) by inbox.vuxu.org with ESMTP; 14 Mar 2020 03:29:45 -0000 Received: (qmail 11812 invoked by alias); 14 Mar 2020 03:29:30 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 45555 Received: (qmail 7480 invoked by uid 1010); 14 Mar 2020 03:29:30 -0000 X-Qmail-Scanner-Diagnostics: from out4-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.2/25744. spamassassin: 3.4.2. Clear:RC:0(66.111.4.28):SA:0(-2.6/5.0):. Processed in 5.047192 secs); 14 Mar 2020 03:29:30 -0000 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedruddvkedgiedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkjghfofggtgesmhdttd erredtjeenucfhrhhomhepffgrnhhivghlucfuhhgrhhgrfhcuoegurdhssegurghnihgv lhdrshhhrghhrghfrdhnrghmvgeqnecukfhppedutdelrdeigedrvddvrdduieeknecuve hluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepugdrshesuggr nhhivghlrdhshhgrhhgrfhdrnhgrmhgv X-ME-Proxy: Date: Sat, 14 Mar 2020 03:28:45 +0000 From: Daniel Shahaf To: zsh-workers@zsh.org Subject: Re: Unset =?UTF-8?B?4oCcemxlX2JyYWNrZXRlZF9wYXN0ZeKAnQ==?= .zshrc Message-ID: <20200314032845.2b2930e5@tarpaulin.shahaf.local2> In-Reply-To: <1580998165.14726.20.camel@samsung.com> References: <0723EF0A-BD62-4C2C-AAA1-735AD3D64768@icloud.com> <20200123031249.034c7e1a@tarpaulin.shahaf.local2> <1579773213.5343.1.camel@samsung.com> <20200126004519.6e44ae68@tarpaulin.shahaf.local2> <1580133672.4960.15.camel@samsung.com> <1580209759.6442.3.camel@samsung.com> <1580297021.5380.17.camel@samsung.com> <20200206124023.450ee3f3@tarpaulin.shahaf.local2> <1580998165.14726.20.camel@samsung.com> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/Rh7LUgp+p6uH4bngJIOu0aJ" --MP_/Rh7LUgp+p6uH4bngJIOu0aJ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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 wrote: =20 > > >=C2=A0 > > > Well, it's an option.=C2=A0=C2=A0It would result in the following int= erface: > > > to prevent $zle_bracketed_paste from being defined one will have to r= un > > > =C2=ABunset zle_bracketed_paste=C2=BB if zsh/zle has been loaded, or = =C2=ABzmodload -Fa > > > zsh/zle -p:zle_bracketed_paste=C2=BB otherwise. =20 > > 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 =20 >=20 > No, this is entirely about autoload behaviour. =C2=A0"zmodload -Fa" provi= des > selective autoload out of the box, which in principle fixes the issue > with no fundamentally new features, just appropriate module exports. >=20 > 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. =C2=A0So in practice adding behaviour to "unset" is probably > easier for everyone. >=20 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? =C2=A0But that can go way down a = very > long list. >=20 > pws >=20 --MP_/Rh7LUgp+p6uH4bngJIOu0aJ Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=patch.txt 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} --MP_/Rh7LUgp+p6uH4bngJIOu0aJ--