zsh-workers
 help / color / mirror / code / Atom feed
* Unset “zle_bracketed_paste” .zshrc
@ 2020-01-17 23:20 Andrew Reyes
  2020-01-18 19:40 ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Reyes @ 2020-01-17 23:20 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 178 bytes --]

The problem is: 
1) zsh/zle's setup_() assigns the parameter, overwriting existing values

2) not possible to 'unset' the parameter before zsh/zle is loaded.

Sent from my iPhone

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-17 23:20 Unset “zle_bracketed_paste” .zshrc Andrew Reyes
@ 2020-01-18 19:40 ` Daniel Shahaf
  2020-01-23  3:12   ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2020-01-18 19:40 UTC (permalink / raw)
  To: Andrew Reyes, zsh-workers

Andrew Reyes wrote on Fri, 17 Jan 2020 23:20 +00:00:
> The problem is: 
> 1) zsh/zle's setup_() assigns the parameter, overwriting existing values
> 
> 2) not possible to 'unset' the parameter before zsh/zle is loaded.

@Andrew thanks for the report.  Let me spell out the symptoms for the list:

In zshrc, «typeset -a zle_bracketed_paste=('' '')» and «unset
zle_bracketed_paste» have no effect if done before zsh/zle has been
loaded, because of zsh/zle's setup_() unconditionally assigns to the
parameter, even if it already exists and has a non-null value.

I looked into this yesterday, and guarding the assignaparam() call with
a paramtab->getnode() != NULL fixes the case that the variable is
already set by the time zsh/zle is loaded (e.g., if it's set in zshrc).
However, I haven't been able to make «unset zle_bracketed_paste»  have
any lasting effect if done before zsh/zle is loaded.  Is that achievable?


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-18 19:40 ` Daniel Shahaf
@ 2020-01-23  3:12   ` Daniel Shahaf
  2020-01-23  9:53     ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2020-01-23  3:12 UTC (permalink / raw)
  To: zsh-workers; +Cc: Andrew Reyes

[tl;dr: See the antepenultimate paragraph.]

Daniel Shahaf wrote on Sat, 18 Jan 2020 19:40:42 +0000:
> Andrew Reyes wrote on Fri, 17 Jan 2020 23:20 +00:00:
> > The problem is: 
> > 1) zsh/zle's setup_() assigns the parameter, overwriting existing values
> > 
> > 2) not possible to 'unset' the parameter before zsh/zle is loaded.
> 
> @Andrew thanks for the report.  Let me spell out the symptoms for the list:
> 
> In zshrc, «typeset -a zle_bracketed_paste=('' '')» and «unset
> zle_bracketed_paste» have no effect if done before zsh/zle has been
> loaded, because of zsh/zle's setup_() unconditionally assigns to the
> parameter, even if it already exists and has a non-null value.
> 
> I looked into this yesterday, and guarding the assignaparam() call with
> a paramtab->getnode() != NULL fixes the case that the variable is
> already set by the time zsh/zle is loaded (e.g., if it's set in zshrc).
> However, I haven't been able to make «unset zle_bracketed_paste»  have
> any lasting effect if done before zsh/zle is loaded.  Is that achievable?

I think it is achievable.

We already have a facility for knowing which parameters will be provided
by not-yet-loaded modules:

    $ zsh -fc 'zmodload -ap; zmodload' | tail
    patchars (zsh/parameter)
    reswords (zsh/parameter)
    saliases (zsh/parameter)
    termcap (zsh/termcap)
    terminfo (zsh/terminfo)
    userdirs (zsh/parameter)
    usergroups (zsh/parameter)
    widgets (zsh/zleparameter)
    zsh_scheduled_events (zsh/sched)
    zsh/main
    $ 

(For those following along at home: the 'autofeatures' array in the
*.mdd file is compiled into a call to the autofeatures() function, which
is done from init_bltinmods() during shell initialization.)

Currently, unsetting a marked-for-autoloading parameter removes all
traces of it:

    $ zsh -fc 'unset aliases; zmodload -ap | grep -w aliases; zmodload'
    zsh/main

… but it's silently recreated when the module is actually loaded:

    $ zsh -fc 'unset aliases; zmodload zsh/parameter; echo ${(t)aliases}' 
    association-hide-hideval-special

Because we know which parameters will be provided by modules _before_
those modules are loaded, we can change the behaviour so loading
a module would not create parameters that had been marked autoloadable
and subsequently unset; that is, we can make it so after «unset aliases;
zmodload zsh/parameter» the parameter $aliases would remain unset.
Would this be the Right way to proceed?

Or would it be better to keep the current behaviour and document that modules
must be loaded before parameters provided by them are «unset»?  (In this
case, we could still check whether $zle_bracketed_paste exists before
silently overwriting its zshrc-set value.)

I haven't tried to implement this yet, but what I have in mind is
(1) Make unsetparam_pm() add the PM_UNSET bit if the PM_AUTOLOAD bit is
present; (2) Make module loading, before creating a parameter, check if
there's a Param with PM_UNSET and PM_AUTOLOAD both set; if there is,
rather than create the "real" parameter, delete the tombstone parameter.
(But if there isn't a Param at all, the module _should_ create its special
Param anyway, to allow modules to be unloaded and reloaded in the same
shell session.)

Thanks,

Daniel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-23  3:12   ` Daniel Shahaf
@ 2020-01-23  9:53     ` Peter Stephenson
  2020-01-26  0:45       ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2020-01-23  9:53 UTC (permalink / raw)
  To: zsh-workers

On Thu, 2020-01-23 at 03:12 +0000, Daniel Shahaf wrote:
> I haven't tried to implement this yet, but what I have in mind is
> (1) Make unsetparam_pm() add the PM_UNSET bit if the PM_AUTOLOAD bit is
> present; (2) Make module loading, before creating a parameter, check if
> there's a Param with PM_UNSET and PM_AUTOLOAD both set; if there is,
> rather than create the "real" parameter, delete the tombstone parameter.
> (But if there isn't a Param at all, the module _should_ create its special
> Param anyway, to allow modules to be unloaded and reloaded in the same
> shell session.)

The obvious expected behaviour would be for it to have the same
behavious as unsetting the parameter after the module is loaded.  But a
quick tests suggests that doesn't work for readonly parameters, for one
--- all that would happen is that would produce an error when the module
is loaded, which doesn't really make much sense.  So in that case, at
least, the behaviour above is as logical as anything.

Documenting the current behaviour is perfectly respectable.  The
autoload flag effective means the parameter behaviour defers to the
module.  Remember, you can use zmodload -F to manipulate which features
are provided by a module, although not at the point of declaring
autoloads --- some sort of autoloadable feature set might be another
way of doing this.

pws


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-23  9:53     ` Peter Stephenson
@ 2020-01-26  0:45       ` Daniel Shahaf
  2020-01-27 14:01         ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2020-01-26  0:45 UTC (permalink / raw)
  To: zsh-workers

[ Sorry for the delay; I wanted to sleep on this before sending. ]

Peter Stephenson wrote on Thu, 23 Jan 2020 09:53 +0000:
> On Thu, 2020-01-23 at 03:12 +0000, Daniel Shahaf wrote:
> > I haven't tried to implement this yet, but what I have in mind is
> > (1) Make unsetparam_pm() add the PM_UNSET bit if the PM_AUTOLOAD bit is
> > present; (2) Make module loading, before creating a parameter, check if
> > there's a Param with PM_UNSET and PM_AUTOLOAD both set; if there is,
> > rather than create the "real" parameter, delete the tombstone parameter.
> > (But if there isn't a Param at all, the module _should_ create its special
> > Param anyway, to allow modules to be unloaded and reloaded in the same
> > shell session.)  
> 
> The obvious expected behaviour would be for it to have the same
> behavious as unsetting the parameter after the module is loaded.  But a
> quick tests suggests that doesn't work for readonly parameters, for one
> --- all that would happen is that would produce an error when the module
> is loaded, which doesn't really make much sense.

In what case would an error happen upon loading of the module?

The only error I see is this:

    % zsh -fc 'unset builtins; zmodload zsh/parameter'
    %

    % zsh -fc 'zmodload zsh/parameter; unset builtins' 
    zsh:1: read-only variable: builtins
    zsh: exit 1     zsh -fc 'zmodload zsh/parameter; unset builtins'
    % 

And I think it makes sense, because trying to unset a non-autoloadable
readonly parameter gives the same error.

> So in that case, at least, the behaviour above is as logical as
> anything.
> 
> [...] The autoload flag effective means the parameter behaviour defers to
> the module.

In other words, the rule is a parameter should only be unset by
explicitly calling the «unset» builtin; loading and unloading the module
doesn't affect the parameter's existence.  Thanks, this makes sense.

How about the following spec? —

[[[
diff --git a/Test/V01zmodload.ztst b/Test/V01zmodload.ztst
index 1bd8c1900..854e21da0 100644
--- a/Test/V01zmodload.ztst
+++ b/Test/V01zmodload.ztst
@@ -348,6 +348,47 @@
 ?(eval):6: unknown function: systell
 ?(eval):9: file descriptor out of range
 
+ $ZTST_testdir/../Src/zsh -fc '
+   if zmodload -e zsh/parameter; then zmodload -u zsh/parameter; fi
+   unset options
+   zmodload zsh/parameter
+   echo $+options
+ '
+-f:can unset a non-readonly autoloadable parameter before loading the module
+>0
+# Currently prints '1'.
+
+ $ZTST_testdir/../Src/zsh -fc '
+   zmodload zsh/parameter
+   unset options
+   echo $+options
+ '
+0:can unset a non-readonly autoloadable parameter after loading the module
+>0
+
+ $ZTST_testdir/../Src/zsh -fc '
+   if zmodload -e zsh/parameter; then zmodload -u zsh/parameter; fi
+   unset builtins
+ '
+-f:can't unset a readonly autoloadable parameter before loading the module
+*?zsh:?: read-only variable: builtins
+# Currently, the 'unset' succeeds.
+
+ $ZTST_testdir/../Src/zsh -fc '
+   zmodload zsh/parameter
+   unset builtins
+ '
+1:can't unset a readonly autoloadable parameter after loading the module
+?zsh:3: read-only variable: builtins
+
+ $ZTST_testdir/../Src/zsh -fc '
+   zmodload zsh/parameter
+   zmodload -u zsh/parameter
+   echo $options
+ '
+0:unloading a module doesn't implicitly unset autoloadable parameters
+*>(on|off) *
+
 %clean
 
  eval "$deps"
]]]

(Note that two of the five cases currently fail.)

> Documenting the current behaviour is perfectly respectable.

We could do this for 5.8, 
but I'd like to take a shot at the
general case too.  (Though I already have a backlog of patches I want to
finish, or in some cases start.)

> Remember, you can use zmodload -F to manipulate which features
> are provided by a module, although not at the point of declaring
> autoloads --- some sort of autoloadable feature set might be another
> way of doing this.

How do you envision this working?  Would there be, say, a zmodload flag
to add/remove entries from the default set of autofeatures?  Or would
«unset options» implicitly twiddle the autofeatures metadata for the
(not-yet-loaded) zsh/parameter module?

Thanks,

Daniel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-26  0:45       ` Daniel Shahaf
@ 2020-01-27 14:01         ` Peter Stephenson
  2020-01-28 11:09           ` Peter Stephenson
  2020-01-29  8:34           ` Daniel Shahaf
  0 siblings, 2 replies; 13+ messages in thread
From: Peter Stephenson @ 2020-01-27 14:01 UTC (permalink / raw)
  To: zsh-workers

On Sun, 2020-01-26 at 00:45 +0000, Daniel Shahaf wrote:
> > The obvious expected behaviour would be for it to have the same
> > behavious as unsetting the parameter after the module is loaded.  But a
> > quick tests suggests that doesn't work for readonly parameters, for one
> > --- all that would happen is that would produce an error when the module
> > is loaded, which doesn't really make much sense.
> In what case would an error happen upon loading of the module?
>
> The only error I see is this:
> 
>     % zsh -fc 'unset builtins; zmodload zsh/parameter'
>     %
> 
>     % zsh -fc 'zmodload zsh/parameter; unset builtins' 
>     zsh:1: read-only variable: builtins
>     zsh: exit 1     zsh -fc 'zmodload zsh/parameter; unset builtins'
>     % 
> 
> And I think it makes sense, because trying to unset a non-autoloadable
> readonly parameter gives the same error.

The worry is simply that this happens on any autoload to that module,
which may be triggered by a completely different parameter; you then see
this message about a parameter that's got nothing to do with the
operation that's actually taking place.

So if does what you expect, so long as you don't expect it to be
seamless and with no possible confusion :-).

> > [...] The autoload flag effective means the parameter behaviour defers to
> > the module.
> In other words, the rule is a parameter should only be unset by
> explicitly calling the «unset» builtin; loading and unloading the module
> doesn't affect the parameter's existence.  Thanks, this makes sense.
> 
> How about the following spec? —

Those look sensible --- which I think is all we can ask for, I don't see
any hard and fast rules coming down from on high.  <Peers quickly upwards>

> > Documenting the current behaviour is perfectly respectable.
> We could do this for 5.8, 
> but I'd like to take a shot at the
> general case too.  (Though I already have a backlog of patches I want to
> finish, or in some cases start.)

I'm not going to argue, either way.

> > Remember, you can use zmodload -F to manipulate which features
> > are provided by a module, although not at the point of declaring
> > autoloads --- some sort of autoloadable feature set might be another
> > way of doing this.
> How do you envision this working?  Would there be, say, a zmodload flag
> to add/remove entries from the default set of autofeatures?  Or would
> «unset options» implicitly twiddle the autofeatures metadata for the
> (not-yet-loaded) zsh/parameter module?

I'd start by simply add the interface to zmodload itself, in the first
instances.  That's already quite a job and not clear how useful it is.
At the moment, until you can read the feature set from the module you're
just guessing what the module provides.  The obvious fix is simply let
the user claim e.g. module zsh/foo provides p:bar and complain if it
doesn't when the module is loaded.

Any consequent interaction between parameter- or other feature-specific
code would then be a further issue on top of that.  So if the above
already looks a bit clunky, we can bury this idea.

pws


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-27 14:01         ` Peter Stephenson
@ 2020-01-28 11:09           ` Peter Stephenson
  2020-01-29 11:23             ` Peter Stephenson
  2020-01-29  8:34           ` Daniel Shahaf
  1 sibling, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2020-01-28 11:09 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2020-01-27 at 14:01 +0000, Peter Stephenson wrote:
> On Sun, 2020-01-26 at 00:45 +0000, Daniel Shahaf wrote:
>>> Remember, you can use zmodload -F to manipulate which features
>>> are provided by a module, although not at the point of declaring
>>> autoloads --- some sort of autoloadable feature set might be another
>>> way of doing this.
>>
>> How do you envision this working?  Would there be, say, a zmodload flag
>> to add/remove entries from the default set of autofeatures?  Or would
>> «unset options» implicitly twiddle the autofeatures metadata for the
>> (not-yet-loaded) zsh/parameter module?
>
> I'd start by simply add the interface to zmodload itself, in the first
> instances.  That's already quite a job and not clear how useful it is.
> At the moment, until you can read the feature set from the module you're
> just guessing what the module provides.  The obvious fix is simply let
> the user claim e.g. module zsh/foo provides p:bar and complain if it
> doesn't when the module is loaded.

Duh.  We already have this --- "zmodload -Fa", and it's already
documented as fixing the problem that was bugging me --- don't
automatically load other features from the module as that can cause
unwanted side effects.  I'd completely forgotten implementing this.

% zmodload -Fa zsh/datetime p:EPOCHSECONDS
% print $EPOCHSECONDS
1580209515
% zmodload -Fl zsh/datetime
-b:strftime
+p:EPOCHSECONDS
-p:EPOCHREALTIME
-p:epochtime

This is another way of fixing the underlying problem --- e.g. here
you don't need to "unset EPOCHREALTIME" because it wasn't provided
as a feature in the first place.  You can turn on and off the
feature as needed once the module is loaded.

pws


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-27 14:01         ` Peter Stephenson
  2020-01-28 11:09           ` Peter Stephenson
@ 2020-01-29  8:34           ` Daniel Shahaf
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel Shahaf @ 2020-01-29  8:34 UTC (permalink / raw)
  To: zsh-workers

Peter Stephenson wrote on Mon, 27 Jan 2020 14:01 +0000:
> On Sun, 2020-01-26 at 00:45 +0000, Daniel Shahaf wrote:
> > > [...] The autoload flag effective means the parameter behaviour defers to
> > > the module.  
> > In other words, the rule is a parameter should only be unset by
> > explicitly calling the «unset» builtin; loading and unloading the module
> > doesn't affect the parameter's existence.  Thanks, this makes sense.
> > 
> > How about the following spec? —  
> 
> Those look sensible --- which I think is all we can ask for, I don't see
> any hard and fast rules coming down from on high.  <Peers quickly upwards>

Thanks, committed.  I'll think about the rest — hopefully come back with
a patch, once I've had time to write it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-28 11:09           ` Peter Stephenson
@ 2020-01-29 11:23             ` Peter Stephenson
  2020-02-06 12:40               ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2020-01-29 11:23 UTC (permalink / raw)
  To: zsh-workers

On Tue, 2020-01-28 at 11:09 +0000, Peter Stephenson wrote:
> % zmodload -Fa zsh/datetime p:EPOCHSECONDS
> % print $EPOCHSECONDS
> 1580209515
> % zmodload -Fl zsh/datetime
> -b:strftime
> +p:EPOCHSECONDS
> -p:EPOCHREALTIME
> -p:epochtime
> 
> This is another way of fixing the underlying problem --- e.g. here
> you don't need to "unset EPOCHREALTIME" because it wasn't provided
> as a feature in the first place.  You can turn on and off the
> feature as needed once the module is loaded.

I see parameters used by ZLE are only fairly weakly linked into this
mechanism, however.  At least one reason for that is because of the
unhelpfully tight linkage between ZLE and the main shell, so it's
not simply a straightforward plugin.  However, I haven't looked
further so I don't know if that's an issue with zle_bracketed_paste.
There may be some scope for rationalisation.

Loading a module for your own purposes --- for which Sebastian
no doubt provides a good deal of support --- is in any case a
rather different thing from the shell loading a module to provide
its own basic features, so this may not be going anywhere particularly
useful in this case.

pws


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-01-29 11:23             ` Peter Stephenson
@ 2020-02-06 12:40               ` Daniel Shahaf
  2020-02-06 13:32                 ` Mikael Magnusson
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Shahaf @ 2020-02-06 12:40 UTC (permalink / raw)
  To: zsh-workers

Peter Stephenson wrote on Wed, 29 Jan 2020 11:23 +0000:
> On Tue, 2020-01-28 at 11:09 +0000, Peter Stephenson wrote:
> > % zmodload -Fa zsh/datetime p:EPOCHSECONDS
> > % print $EPOCHSECONDS
> > 1580209515
> > % zmodload -Fl zsh/datetime
> > -b:strftime
> > +p:EPOCHSECONDS
> > -p:EPOCHREALTIME
> > -p:epochtime
> > 
> > This is another way of fixing the underlying problem --- e.g. here
> > you don't need to "unset EPOCHREALTIME" because it wasn't provided
> > as a feature in the first place.  You can turn on and off the
> > feature as needed once the module is loaded.  
> 
> I see parameters used by ZLE are only fairly weakly linked into this
> mechanism, however.  At least one reason for that is because of the
> unhelpfully tight linkage between ZLE and the main shell, so it's
> not simply a straightforward plugin.  However, I haven't looked
> further so I don't know if that's an issue with zle_bracketed_paste.
> There may be some scope for rationalisation.
> 

zle_bracketed_paste isn't declared special; it's just getaparam()'d in
the guts of a random C function.

> Loading a module for your own purposes --- for which Sebastian
> no doubt provides a good deal of support --- is in any case a
> rather different thing from the shell loading a module to provide
> its own basic features, so this may not be going anywhere particularly
> useful in this case.

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.

On the other hand, the approach I described in 45339 aimed to make it
possible to just do «unset zle_bracketed_paste» to unset that parameter
(and keep it unset), regardless of whether zsh/zle has or hasn't been
loaded.

I find the latter option preferable.  (Assuming it can be implemented
without errors about readonly parameters cropping up at unexpected
times, as discussed upthread)

In the meantime, here's a test for -Fa.

diff --git a/Test/V01zmodload.ztst b/Test/V01zmodload.ztst
index 1bd8c1900..6a4e34d2d 100644
--- a/Test/V01zmodload.ztst
+++ b/Test/V01zmodload.ztst
@@ -348,6 +348,20 @@
 ?(eval):6: unknown function: systell
 ?(eval):9: file descriptor out of range
 
+ $ZTST_testdir/../Src/zsh -fc '
+  zmodload zsh/zutil
+  zmodload -Fal zsh/zutil | grep parse
+  zmodload -u zsh/zutil
+  #
+  zmodload -Fa zsh/zutil -b:zregexparse
+  zmodload zsh/zutil
+  zmodload -Fal zsh/zutil | grep parse >&2
+ '
+0:zmodload -Fa can disable features from being loaded
+>b:zparseopts
+>b:zregexparse
+?b:zparseopts
+
 %clean
 
  eval "$deps"

Sorry for the delays responding to this thread; life has been getting
in the way.

Daniel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-02-06 12:40               ` Daniel Shahaf
@ 2020-02-06 13:32                 ` Mikael Magnusson
  2020-02-06 14:09                   ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Mikael Magnusson @ 2020-02-06 13:32 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> 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

-- 
Mikael Magnusson

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-02-06 13:32                 ` Mikael Magnusson
@ 2020-02-06 14:09                   ` Peter Stephenson
  2020-03-14  3:28                     ` Daniel Shahaf
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2020-02-06 14:09 UTC (permalink / raw)
  To: zsh-workers

On Thu, 2020-02-06 at 14:32 +0100, Mikael Magnusson wrote:
> On 2/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> 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.

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Unset “zle_bracketed_paste” .zshrc
  2020-02-06 14:09                   ` Peter Stephenson
@ 2020-03-14  3:28                     ` Daniel Shahaf
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Shahaf @ 2020-03-14  3:28 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]

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@daniel.shahaf.name> 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
> 


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 6328 bytes --]

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}

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-03-14  3:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 23:20 Unset “zle_bracketed_paste” .zshrc Andrew Reyes
2020-01-18 19:40 ` Daniel Shahaf
2020-01-23  3:12   ` Daniel Shahaf
2020-01-23  9:53     ` Peter Stephenson
2020-01-26  0:45       ` Daniel Shahaf
2020-01-27 14:01         ` Peter Stephenson
2020-01-28 11:09           ` Peter Stephenson
2020-01-29 11:23             ` Peter Stephenson
2020-02-06 12:40               ` Daniel Shahaf
2020-02-06 13:32                 ` Mikael Magnusson
2020-02-06 14:09                   ` Peter Stephenson
2020-03-14  3:28                     ` Daniel Shahaf
2020-01-29  8:34           ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).