zsh-workers
 help / color / mirror / code / Atom feed
* [BUG] Can't mark unset variables as read-only
@ 2015-04-29  6:35 Martijn Dekker
  2015-04-29 10:36 ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Martijn Dekker @ 2015-04-29  6:35 UTC (permalink / raw)
  To: zsh-workers

Unlike other shells, zsh can't mark an unset variable as read-only.
Upon encountering the readonly command, the variable is set to the empty
string instead.

Test code:

( unset -v testv && readonly testv && test "${testv+set}" = '' ) || \
        echo "*** Can't keep unset variables as read-only." 1>&2

This succeeds in ash, dash, bash, ksh, etc. but fails in zsh, even in
'emulate sh' mode.

Actual result:
testv is set to the empty string and marked read-only.

Expected result:
testv remains unset and is marked read-only.

Real-world impact:
Testing whether a variable is set is a common way of checking whether a
certain setting or feature is active. Hardened scripts may want to set
such either-set-or-unset variables as read-only after setting up their
environment, to prevent them from being accidentally clobbered. This is
not possible in zsh.


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29  6:35 [BUG] Can't mark unset variables as read-only Martijn Dekker
@ 2015-04-29 10:36 ` Peter Stephenson
  2015-04-29 10:57   ` Mikael Magnusson
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Peter Stephenson @ 2015-04-29 10:36 UTC (permalink / raw)
  To: zsh-workers

On Wed, 29 Apr 2015 08:35:43 +0200
Martijn Dekker <martijn@inlv.org> wrote:
> Unlike other shells, zsh can't mark an unset variable as read-only.

Yes, the standard does indeed require that ability.  As you can imagine,
something like this that completely breaks the normal programming model
of variables (a variable can't have state if it's unset because it
doesn't exist) is a nightmare to implement; however, it can at least be
limited to the POSIXBUILTINS option and maybe the cases using that are
simple enough that it mostly works.  I'm sure there are any number of
strange edge cases, though.

The output of "readonly -p" is still broken (doesn't show the unset
variables in a fashion that can be used for restoring the current state)
but it was anyway:

  typeset -ar '*'
  *=()

Er, no, I don't think that's going to work.

So that can be fixed separately.

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index dd5a80f..985d19e 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1933,6 +1933,13 @@ item(tt(-r))(
 The given var(name)s are marked readonly.  Note that if var(name) is a
 special parameter, the readonly attribute can be turned on, but cannot then
 be turned off.
+
+If the tt(POSIX_BUILTINS) option is set, the readonly attribute is
+more restrictive: unset variables can be marked readonly and cannot then
+be set; furthermore, the readonly attribute cannot be removed from any
+variable.  Note that in zsh (unlike other shells) it is still possible
+to create a local variable of the same name as this is considered a
+different variable (though this variable, too, can be marked readonly).
 )
 item(tt(-t))(
 Tags the named parameters.  Tags have no special meaning to the shell.
diff --git a/Src/builtin.c b/Src/builtin.c
index de01014..0a57489 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1931,8 +1931,12 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
      * locallevel as an unset one we use the pm struct anyway: that's
      * handled in createparam().  Here we just avoid using it for the
      * present tests if it's unset.
+     *
+     * POSIXBUILTINS horror: we need to retain the 'readonly' flag
+     * of an unset parameter.
      */
-    usepm = pm && !(pm->node.flags & PM_UNSET);
+    usepm = pm && (!(pm->node.flags & PM_UNSET) ||
+		   (isset(POSIXBUILTINS) && (pm->node.flags & PM_READONLY)));
 
     /*
      * We need to compare types with an existing pm if special,
@@ -2032,6 +2036,20 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
     else if (newspecial != NS_NONE && strcmp(pname, "SECONDS") == 0)
 	newspecial = NS_SECONDS;
 
+    if (isset(POSIXBUILTINS)) {
+	/*
+	 * Stricter rules about retaining readonly attribute in this case.
+	 */
+	if ((on & PM_READONLY) && (!usepm || (pm->node.flags & PM_UNSET)) &&
+	    !value)
+	    on |= PM_UNSET;
+	else if (usepm && (pm->node.flags & PM_READONLY) &&
+		 !(on & PM_READONLY)) {
+	    zerr("read-only variable: %s", pm->node.nam);
+	    return NULL;
+	}
+    }
+
     /*
      * A parameter will be local if
      * 1. we are re-using an existing local parameter
@@ -2078,9 +2096,15 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	}
 	if (usepm == 2)		/* do not change the PM_UNSET flag */
 	    pm->node.flags = (pm->node.flags | (on & ~PM_READONLY)) & ~off;
-	else
+	else {
+	    /*
+	     * Keep unset if using readonly in POSIX mode.
+	     */
+	    if (!(on & PM_READONLY) || !isset(POSIXBUILTINS))
+		off |= PM_UNSET;
 	    pm->node.flags = (pm->node.flags |
-			      (on & ~PM_READONLY)) & ~(off | PM_UNSET);
+			      (on & ~PM_READONLY)) & ~off;
+	}
 	if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
 	    if (typeset_setwidth(cname, pm, ops, on, 0))
 		return NULL;
@@ -2256,7 +2280,12 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	 * readonly flag
 	 */
 	pm = createparam(pname, on & ~PM_READONLY);
-	DPUTS(!pm, "BUG: parameter not created");
+	if (!pm) {
+	    if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z |
+		      PM_INTEGER | PM_EFLOAT | PM_FFLOAT))
+		zerrnam(cname, "can't change variable attribute: %s", pname);
+	    return NULL;
+	}
 	if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) {
 	    if (typeset_setwidth(cname, pm, ops, on, 0))
 		return NULL;
diff --git a/Src/params.c b/Src/params.c
index e8a9010..d53b6ca 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -874,10 +874,14 @@ createparam(char *name, int flags)
 	DPUTS(oldpm && oldpm->level > locallevel,
 	      "BUG: old local parameter not deleted");
 	if (oldpm && (oldpm->level == locallevel || !(flags & PM_LOCAL))) {
+	    if (isset(POSIXBUILTINS) && (oldpm->node.flags & PM_READONLY)) {
+		zerr("read-only variable: %s", name);
+		return NULL;
+	    }
 	    if (!(oldpm->node.flags & PM_UNSET) || (oldpm->node.flags & PM_SPECIAL)) {
 		oldpm->node.flags &= ~PM_UNSET;
 		if ((oldpm->node.flags & PM_SPECIAL) && oldpm->ename) {
-		    Param altpm = 
+		    Param altpm =
 			(Param) paramtab->getnode(paramtab, oldpm->ename);
 		    if (altpm)
 			altpm->node.flags &= ~PM_UNSET;
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 51ebc65..f4fb8ec 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -468,3 +468,20 @@
 0:retying arrays to same array works
 >foo bar
 >goo car
+
+ (
+   setopt POSIXBUILTINS
+   readonly pbro
+   print ${+pbro} >&2
+   (typeset pbro=3)
+   (pbro=4)
+   typeset -r pbro        # idempotent (no error)...
+   print ${+pbro} >&2     # ...so still readonly...
+   typeset +r pbro        # ...can't turn it off
+ )
+1:Readonly with POSIX_BUILTINS
+?0
+?(eval):5: read-only variable: pbro
+?(eval):6: read-only variable: pbro
+?0
+?(eval):9: read-only variable: pbro


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 10:36 ` Peter Stephenson
@ 2015-04-29 10:57   ` Mikael Magnusson
  2015-04-29 11:01     ` Mikael Magnusson
  2015-04-29 11:09     ` Local readonly specials (was: Can't mark unset variables as read-only) Peter Stephenson
  2015-04-29 13:46   ` [BUG] Can't mark unset variables as read-only Martijn Dekker
  2015-04-29 13:55   ` Bart Schaefer
  2 siblings, 2 replies; 16+ messages in thread
From: Mikael Magnusson @ 2015-04-29 10:57 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh workers

On Wed, Apr 29, 2015 at 12:36 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 29 Apr 2015 08:35:43 +0200
> Martijn Dekker <martijn@inlv.org> wrote:
>> Unlike other shells, zsh can't mark an unset variable as read-only.
>
> Yes, the standard does indeed require that ability.  As you can imagine,
> something like this that completely breaks the normal programming model
> of variables (a variable can't have state if it's unset because it
> doesn't exist) is a nightmare to implement; however, it can at least be
> limited to the POSIXBUILTINS option and maybe the cases using that are
> simple enough that it mostly works.  I'm sure there are any number of
> strange edge cases, though.
>
> The output of "readonly -p" is still broken (doesn't show the unset
> variables in a fashion that can be used for restoring the current state)
> but it was anyway:
>
>   typeset -ar '*'
>   *=()
>
> Er, no, I don't think that's going to work.
>
> So that can be fixed separately.

I wanted the opposite thing the other day, sort of. I have a ZLE
widget that looks at $WIDGET, and I wanted to reuse it by calling it
from another widget, setting WIDGET to another value first. However,
ZLE makes $WIDGET readonly special, and I couldn't find any
combination of flags that let me make a non-readonly local WIDGET. Is
that possible?

% zle -N h; h() { local -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
% <press alt-h>
h: read-only variable: WIDGET
% zle -N h; h() { local +r -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
% <press alt-h>
h:local: WIDGET: can't change type of a special parameter
% zle -N h; h() { local WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
% <press alt-h>WIDGET=h

h: read-only variable: WIDGET

compared to
% () { readonly foo=3; () { local foo=5; echo $foo }; echo $foo }
5
3

-- 
Mikael Magnusson


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 10:57   ` Mikael Magnusson
@ 2015-04-29 11:01     ` Mikael Magnusson
  2015-04-30  0:08       ` PATCH: Don't define internal params directly in hook function scope Mikael Magnusson
  2015-04-29 11:09     ` Local readonly specials (was: Can't mark unset variables as read-only) Peter Stephenson
  1 sibling, 1 reply; 16+ messages in thread
From: Mikael Magnusson @ 2015-04-29 11:01 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh workers

On Wed, Apr 29, 2015 at 12:57 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On Wed, Apr 29, 2015 at 12:36 PM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
>> On Wed, 29 Apr 2015 08:35:43 +0200
>> Martijn Dekker <martijn@inlv.org> wrote:
>>> Unlike other shells, zsh can't mark an unset variable as read-only.
>>
>> Yes, the standard does indeed require that ability.  As you can imagine,
>> something like this that completely breaks the normal programming model
>> of variables (a variable can't have state if it's unset because it
>> doesn't exist) is a nightmare to implement; however, it can at least be
>> limited to the POSIXBUILTINS option and maybe the cases using that are
>> simple enough that it mostly works.  I'm sure there are any number of
>> strange edge cases, though.
>>
>> The output of "readonly -p" is still broken (doesn't show the unset
>> variables in a fashion that can be used for restoring the current state)
>> but it was anyway:
>>
>>   typeset -ar '*'
>>   *=()
>>
>> Er, no, I don't think that's going to work.
>>
>> So that can be fixed separately.
>
> I wanted the opposite thing the other day, sort of. I have a ZLE
> widget that looks at $WIDGET, and I wanted to reuse it by calling it
> from another widget, setting WIDGET to another value first. However,
> ZLE makes $WIDGET readonly special, and I couldn't find any
> combination of flags that let me make a non-readonly local WIDGET. Is
> that possible?
>
> % zle -N h; h() { local -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
> % <press alt-h>
> h: read-only variable: WIDGET
> % zle -N h; h() { local +r -h WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
> % <press alt-h>
> h:local: WIDGET: can't change type of a special parameter
> % zle -N h; h() { local WIDGET; WIDGET=hi; echo $WIDGET }; bindkey '^[h' h
> % <press alt-h>WIDGET=h
>
> h: read-only variable: WIDGET
>
> compared to
> % () { readonly foo=3; () { local foo=5; echo $foo }; echo $foo }
> 5
> 3

Even when you think about something for a couple of days, you don't
realize what the answer is until after you send the question to a
public forum.

ZLE makes the scope of the WIDGET parameter start inside the widget
function already (which is confusing), so it is already local. The
answer is to start another parameter scope first,
% zle -N h; h() { () { local -h WIDGET; WIDGET=hi; echo $WIDGET } };
bindkey '^[h' h
% <press alt-h>hi

-- 
Mikael Magnusson


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

* Local readonly specials  (was: Can't mark unset variables as read-only)
  2015-04-29 10:57   ` Mikael Magnusson
  2015-04-29 11:01     ` Mikael Magnusson
@ 2015-04-29 11:09     ` Peter Stephenson
  1 sibling, 0 replies; 16+ messages in thread
From: Peter Stephenson @ 2015-04-29 11:09 UTC (permalink / raw)
  To: zsh workers

On Wed, 29 Apr 2015 12:57:29 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> I wanted the opposite thing the other day, sort of. I have a ZLE
> widget that looks at $WIDGET, and I wanted to reuse it by calling it
> from another widget, setting WIDGET to another value first. However,
> ZLE makes $WIDGET readonly special, and I couldn't find any
> combination of flags that let me make a non-readonly local WIDGET. Is
> that possible?

Not sure what's going on here, since this usually works:

% typeset -r PATH
% PATH=foo
zsh: read-only variable: PATH
% fn() { typeset -h PATH=foo; print $PATH; }
% fn
foo
% typeset +r PATH
typeset: PATH: can't change type of a special parameter

See if you can make the existing WIDGET hidden (typeset -gh WIDGET?) before
you create a local variable?

pws


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 10:36 ` Peter Stephenson
  2015-04-29 10:57   ` Mikael Magnusson
@ 2015-04-29 13:46   ` Martijn Dekker
  2015-04-29 13:55   ` Bart Schaefer
  2 siblings, 0 replies; 16+ messages in thread
From: Martijn Dekker @ 2015-04-29 13:46 UTC (permalink / raw)
  To: zsh-workers

Peter Stephenson schreef op 29-04-15 om 12:36:
> As you can imagine,
> something like this that completely breaks the normal programming model
> of variables (a variable can't have state if it's unset because it
> doesn't exist) is a nightmare to implement;

FWIW, I imagine a simpler way might be, rather than implementing
read-only as an attribute of variables themselves, to keep a separate
table of variable names marked as read-only which is checked against
upon variable assignment.

I know absolutely nothing about the zsh codebase, so feel free to laugh
at my suggestion. My interest is in getting the most out of
cross-platform shell programming, and I'm really only here to report a bug.

Many thanks for the patch. I'll apply it, test it, and report back.

(Also, my apologies for the initial duplicate. I thought it hadn't gone
through the first time.)

- Martijn


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 10:36 ` Peter Stephenson
  2015-04-29 10:57   ` Mikael Magnusson
  2015-04-29 13:46   ` [BUG] Can't mark unset variables as read-only Martijn Dekker
@ 2015-04-29 13:55   ` Bart Schaefer
  2015-04-29 14:41     ` Peter Stephenson
  2 siblings, 1 reply; 16+ messages in thread
From: Bart Schaefer @ 2015-04-29 13:55 UTC (permalink / raw)
  To: zsh-workers

On Apr 29, 11:36am, Peter Stephenson wrote:
}
} +Note that in zsh (unlike other shells) it is still possible
} +to create a local variable of the same name as this is considered a
} +different variable (though this variable, too, can be marked readonly).

Hrm, I wonder if that, combined with "typeset" automatically creating
locals when called in a function context (also a POSIX incompatibility?) 
defeats the whole purpose of an unset readonly?

Actually it probably undermines readonly for variables that are set, too,
so I probably shouldn't worry about it because it isn't changing.


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 13:55   ` Bart Schaefer
@ 2015-04-29 14:41     ` Peter Stephenson
  2015-04-29 15:33       ` Chet Ramey
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2015-04-29 14:41 UTC (permalink / raw)
  To: zsh-workers

On Wed, 29 Apr 2015 06:55:56 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:

> On Apr 29, 11:36am, Peter Stephenson wrote:
> }
> } +Note that in zsh (unlike other shells) it is still possible
> } +to create a local variable of the same name as this is considered a
> } +different variable (though this variable, too, can be marked readonly).
> 
> Hrm, I wonder if that, combined with "typeset" automatically creating
> locals when called in a function context (also a POSIX incompatibility?) 
> defeats the whole purpose of an unset readonly?

I don't think so... bash makes these local, too:

$ foo=bar
$ fn() { typeset foo=something_else; echo $foo; }
$ fn
something_else
$ echo $foo
bar

So "foo" in fn is logically something entirely different and I don't
think there's any requirement for it to be treated as readonly;
it's entirely a matter for the definition of "fn".

pws


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 14:41     ` Peter Stephenson
@ 2015-04-29 15:33       ` Chet Ramey
  2015-04-29 19:09         ` Stephane Chazelas
  2015-04-30  3:57         ` Bart Schaefer
  0 siblings, 2 replies; 16+ messages in thread
From: Chet Ramey @ 2015-04-29 15:33 UTC (permalink / raw)
  To: Peter Stephenson, zsh-workers; +Cc: chet.ramey

On 4/29/15 10:41 AM, Peter Stephenson wrote:
> On Wed, 29 Apr 2015 06:55:56 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> 
>> On Apr 29, 11:36am, Peter Stephenson wrote:
>> }
>> } +Note that in zsh (unlike other shells) it is still possible
>> } +to create a local variable of the same name as this is considered a
>> } +different variable (though this variable, too, can be marked readonly).
>>
>> Hrm, I wonder if that, combined with "typeset" automatically creating
>> locals when called in a function context (also a POSIX incompatibility?) 

It's not a Posix incompatibility; Posix does not specify typeset or local
variables at all.  The Posix-conformant ways of declaring variables in a
function result in global variables, as Posix specifies.

>> defeats the whole purpose of an unset readonly?

The question is whether or not you can use local variables to `shadow'
readonly variables in a previous scope.  Bash doesn't allow you to do
that.  Given this script:

readonly foo=immutable

func()
{
	local foo=bar
	echo inside: $foo
}

func
echo outside: $foo

Bash gives the following output:

./x26: line 5: local: foo: readonly variable
inside: immutable
outside: immutable

If the purpose of readonly is to make a particular name/value pair
immutable, I think that allowing a local variable to shadow it,
especially if you're going to export that local variable, is a bad thing.

> 
> I don't think so... bash makes these local, too:
> 
> $ foo=bar
> $ fn() { typeset foo=something_else; echo $foo; }
> $ fn
> something_else
> $ echo $foo
> bar
> 
> So "foo" in fn is logically something entirely different and I don't
> think there's any requirement for it to be treated as readonly;

There's no requirement, since there are no standards for local variables
or variable scoping.  Bash does what I think is right.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 15:33       ` Chet Ramey
@ 2015-04-29 19:09         ` Stephane Chazelas
  2015-04-29 23:22           ` Chet Ramey
  2015-04-30  3:57         ` Bart Schaefer
  1 sibling, 1 reply; 16+ messages in thread
From: Stephane Chazelas @ 2015-04-29 19:09 UTC (permalink / raw)
  To: zsh-workers

2015-04-29 11:33:10 -0400, Chet Ramey:
[...]
> func()
> {
> 	local foo=bar
> 	echo inside: $foo
> }
> 
> func
> echo outside: $foo
> 
> Bash gives the following output:
> 
> ./x26: line 5: local: foo: readonly variable
> inside: immutable
> outside: immutable
> 
> If the purpose of readonly is to make a particular name/value pair
> immutable, I think that allowing a local variable to shadow it,
> especially if you're going to export that local variable, is a bad thing.
[...]

I don't agree.

The whole point of having local variables is to avoid problems
with clashing namespaces.


You can have:

f() {
  local foo=1 # f's own variable
  g xxx
  echo "$foo"
}

g() {
  local foo=2
  blah blah
}

(f and g possibly in different libraries)

By declaring foo local, you make sure that you're not clashing
with someone else's foo. 

You don't have to worry on how you name your functions like you
do in POSIX sh (where you'd need to call one variable f_foo, and
the other g_foo for intance).

Above, if you do:

f() {
  readonly foo=2
  g xxx
  echo "$foo"
}

You're basically breaking g for no good reason. f's doesn't want
the value of f to be modified, but g would not have modified it
since it's declaring its *own* foo variable.

Having said that, I don't remember ever using "readonly" in a
script so I can't say I care much.

-- 
Stephane


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 19:09         ` Stephane Chazelas
@ 2015-04-29 23:22           ` Chet Ramey
  0 siblings, 0 replies; 16+ messages in thread
From: Chet Ramey @ 2015-04-29 23:22 UTC (permalink / raw)
  To: Stephane Chazelas, zsh-workers; +Cc: chet.ramey

On 4/29/15 3:09 PM, Stephane Chazelas wrote:
> 2015-04-29 11:33:10 -0400, Chet Ramey:
> [...]
>> func()
>> {
>> 	local foo=bar
>> 	echo inside: $foo
>> }
>>
>> func
>> echo outside: $foo
>>
>> Bash gives the following output:
>>
>> ./x26: line 5: local: foo: readonly variable
>> inside: immutable
>> outside: immutable
>>
>> If the purpose of readonly is to make a particular name/value pair
>> immutable, I think that allowing a local variable to shadow it,
>> especially if you're going to export that local variable, is a bad thing.
> [...]
> 
> I don't agree.

OK, we can disagree.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/


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

* PATCH: Don't define internal params directly in hook function scope
  2015-04-29 11:01     ` Mikael Magnusson
@ 2015-04-30  0:08       ` Mikael Magnusson
  2015-04-30  4:01         ` Bart Schaefer
  0 siblings, 1 reply; 16+ messages in thread
From: Mikael Magnusson @ 2015-04-30  0:08 UTC (permalink / raw)
  To: zsh-workers

Something feels very wrong about this :). I'm not committing it.
(I didn't investigate this at all, it's just based on my observation that wrapping the local in an anonymous function helped, and this also works. It may blow up something).

---
 Src/Zle/compcore.c | 2 ++
 Src/Zle/zle_main.c | 7 ++++++-
 Src/Zle/zle_misc.c | 2 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index f50c9e9..6816847 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -825,6 +825,7 @@ callcompfunc(char *s, char *fn)
 	comp_setunset(rset, (~rset & CP_ALLREALS),
 		      kset, (~kset & CP_ALLKEYS));
 	makezleparams(1);
+	startparamscope();
 	sfcontext = SFC_CWIDGET;
 	NEWHEAPS(compheap) {
 	    LinkList largs = NULL;
@@ -841,6 +842,7 @@ callcompfunc(char *s, char *fn)
 	} OLDHEAPS;
 	sfcontext = osc;
 	endparamscope();
+	endparamscope();
 	lastcmd = 0;
 	incompfunc = icf;
 
diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index ef6a29d..71c7c24 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -1383,12 +1383,14 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 	    }
 	    startparamscope();
 	    makezleparams(0);
+	    startparamscope();
 	    sfcontext = SFC_WIDGET;
 	    opts[XTRACE] = 0;
 	    ret = doshfunc(shf, largs, 1);
 	    opts[XTRACE] = oxt;
 	    sfcontext = osc;
 	    endparamscope();
+	    endparamscope();
 	    lastcmd = 0;
 	    r = 1;
 	    redup(osi, 0);
@@ -1878,6 +1880,7 @@ zlebeforetrap(UNUSED(Hookdef dummy), UNUSED(void *dat))
     if (zleactive) {
 	startparamscope();
 	makezleparams(1);
+	startparamscope();
     }
     return 0;
 }
@@ -1885,8 +1888,10 @@ zlebeforetrap(UNUSED(Hookdef dummy), UNUSED(void *dat))
 static int
 zleaftertrap(UNUSED(Hookdef dummy), UNUSED(void *dat))
 {
-    if (zleactive)
+    if (zleactive) {
+	endparamscope();
 	endparamscope();
+    }
 
     return 0;
 }
diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index 4669ef2..b4e9f0e 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -1609,10 +1609,12 @@ iremovesuffix(ZLE_INT_T c, int keep)
 
 	    startparamscope();
 	    makezleparams(0);
+	    startparamscope();
 	    sfcontext = SFC_COMPLETE;
 	    doshfunc(shfunc, args, 1);
 	    sfcontext = osc;
 	    endparamscope();
+	    endparamscope();
 
 	    if (wasmeta)
 		metafy_line();
-- 
2.2.0.GIT


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

* Re: [BUG] Can't mark unset variables as read-only
  2015-04-29 15:33       ` Chet Ramey
  2015-04-29 19:09         ` Stephane Chazelas
@ 2015-04-30  3:57         ` Bart Schaefer
  1 sibling, 0 replies; 16+ messages in thread
From: Bart Schaefer @ 2015-04-30  3:57 UTC (permalink / raw)
  To: zsh-workers

On Apr 29, 11:33am, Chet Ramey wrote:
} Subject: Re: [BUG] Can't mark unset variables as read-only
}
} > On Wed, 29 Apr 2015 06:55:56 -0700
} > Bart Schaefer <schaefer@brasslantern.com> wrote:
} > 
} >> On Apr 29, 11:36am, Peter Stephenson wrote:
} >> }
} >> } +Note that in zsh (unlike other shells) it is still possible
} >> } +to create a local variable of the same name as this is considered a
} >> } +different variable (though this variable, too, can be marked readonly).
} >>
} >> Hrm, I wonder if that, combined with "typeset" automatically creating
} >> locals when called in a function context (also a POSIX incompatibility?) 
} 
} It's not a Posix incompatibility; Posix does not specify typeset or local
} variables at all.  The Posix-conformant ways of declaring variables in a
} function result in global variables, as Posix specifies.

Thanks for refreshing my memory there.
 
} >> defeats the whole purpose of an unset readonly?
} 
} The question is whether or not you can use local variables to `shadow'
} readonly variables in a previous scope.  Bash doesn't allow you to do
} that.

This discussion has reminded me another thing I forgot:  for variables
that have defined meaning to the shell ("special" variables) zsh allows
shadowing of a readonly special only if the "-h" (hide) flag is passed
to "local" or one of its equivalents (integer, declare, etc.).

} If the purpose of readonly is to make a particular name/value pair
} immutable, I think that allowing a local variable to shadow it,
} especially if you're going to export that local variable, is a bad thing.

Here's a curious data point:  In ksh, functions declared with POSIX
syntax "name() { ...; }" treat readonly from the surrounding scope the
way bash does.  Those declared "function name { ...; }" treat readonly
the way zsh does, allowing shadowing in the local scope.

Maybe zsh needs yet another POSIX_* option.


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

* Re: PATCH: Don't define internal params directly in hook function scope
  2015-04-30  0:08       ` PATCH: Don't define internal params directly in hook function scope Mikael Magnusson
@ 2015-04-30  4:01         ` Bart Schaefer
  2015-04-30  8:44           ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Bart Schaefer @ 2015-04-30  4:01 UTC (permalink / raw)
  To: zsh-workers

On Apr 30,  2:08am, Mikael Magnusson wrote:
}
} Something feels very wrong about this :). I'm not committing it.

I prefer the original approach, if only because it means you have to
think about what you're doing before mucking with things that are
read-only for a reason.

(I believe the answer about adding a new level of scope around the
"local -h" has appeared on zsh-users before, though I can't think of
a good search term to find it.)


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

* Re: PATCH: Don't define internal params directly in hook function scope
  2015-04-30  4:01         ` Bart Schaefer
@ 2015-04-30  8:44           ` Peter Stephenson
  2015-04-30 11:18             ` Peter Stephenson
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Stephenson @ 2015-04-30  8:44 UTC (permalink / raw)
  To: zsh-workers

On Wed, 29 Apr 2015 21:01:40 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Apr 30,  2:08am, Mikael Magnusson wrote:
> }
> } Something feels very wrong about this :). I'm not committing it.
> 
> I prefer the original approach, if only because it means you have to
> think about what you're doing before mucking with things that are
> read-only for a reason.
> 
> (I believe the answer about adding a new level of scope around the
> "local -h" has appeared on zsh-users before, though I can't think of
> a good search term to find it.)

Documenting this is probably a reasonable approach.  Adding a nested
function in the odd case where you need it probably isn't a big issue,
as long as people know.

pws


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

* Re: PATCH: Don't define internal params directly in hook function scope
  2015-04-30  8:44           ` Peter Stephenson
@ 2015-04-30 11:18             ` Peter Stephenson
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Stephenson @ 2015-04-30 11:18 UTC (permalink / raw)
  To: zsh-workers

On Thu, 30 Apr 2015 09:44:04 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> > (I believe the answer about adding a new level of scope around the
> > "local -h" has appeared on zsh-users before, though I can't think of
> > a good search term to find it.)
> 
> Documenting this is probably a reasonable approach.  Adding a nested
> function in the odd case where you need it probably isn't a big issue,
> as long as people know.

For example.

pws

diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
index ffce54c..653678e 100644
--- a/Doc/Zsh/zle.yo
+++ b/Doc/Zsh/zle.yo
@@ -743,6 +743,21 @@ local scope, like parameters created in a function using tt(local).
 Inside completion widgets and traps called while ZLE is active, these
 parameters are available read-only.
 
+Note that the parameters appear as local to any ZLE widget in
+which they appear.  Hence if it is desired to override them this needs
+to be done within a nested function:
+
+example(widget-function+LPAR()+RPAR() {
+  # $WIDGET here refers to the special variable
+  # that is local inside widget-function
+  +LPAR()+RPAR() {
+     # This anonymous nested function allows WIDGET
+     # to be used as a local variable.  The -h
+     # removes the special status of the variable.
+     local -h WIDGET
+  }
+})
+
 startitem()
 vindex(BUFFER)
 item(tt(BUFFER) (scalar))(


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

end of thread, other threads:[~2015-04-30 11:19 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-29  6:35 [BUG] Can't mark unset variables as read-only Martijn Dekker
2015-04-29 10:36 ` Peter Stephenson
2015-04-29 10:57   ` Mikael Magnusson
2015-04-29 11:01     ` Mikael Magnusson
2015-04-30  0:08       ` PATCH: Don't define internal params directly in hook function scope Mikael Magnusson
2015-04-30  4:01         ` Bart Schaefer
2015-04-30  8:44           ` Peter Stephenson
2015-04-30 11:18             ` Peter Stephenson
2015-04-29 11:09     ` Local readonly specials (was: Can't mark unset variables as read-only) Peter Stephenson
2015-04-29 13:46   ` [BUG] Can't mark unset variables as read-only Martijn Dekker
2015-04-29 13:55   ` Bart Schaefer
2015-04-29 14:41     ` Peter Stephenson
2015-04-29 15:33       ` Chet Ramey
2015-04-29 19:09         ` Stephane Chazelas
2015-04-29 23:22           ` Chet Ramey
2015-04-30  3:57         ` Bart Schaefer

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).