Typeset with array

Typeset with array

[Peter Stephenson]

On Fri, 12 Jun 2015 09:42:37 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> > } > }     local cword words=()
> 
> It could, in principle, be very difficultly and time-consumingly
> improved.
> 
> We need in any case to keep the builtin interface to handle cases like
> 
>   local=local
>   $local -i i
> 
> which are valid --- the case
> 
>   $local array=(one two)
> 
> is not valid as there's no keyword to signal special parsing.

Building on the above, I've developed a Cunning Plan so that it's not
too disruptive and is a few days rather than weeks of spare time.

It works like this.

Reserved words are added for typeset synonyms that can handle arrays:
declare, local, readonly, typeset.  On second thoughts I added it to
export which supports "export -T"; it's a bit arbitrary if you can't
assign to the array component in that case, even though export isn't
generally useful for arrays.  For numeric builtins there's no point.

The reserved words trigger variant handling in par_simple(): WC_TYPESET
is very similar to WC_SIMPLE but there's an extra count and set of
assignments tacked on the end.

Here's where the Cunning Plan comes in: the spcial assignment behaviour
only kicks in where there's an actual assignment.  For consistency I did
it for scalars as well as arrays.  So if it comes across something like

  typeset -L2 name foo=bar array=(here there)

then the "-L2 name" is handled in the original way, but the
last two arguments are bundled up as assigments.  This means,
later on, the option parsing code needs no changes at all --
typeset options are simple and can't include arbitrary strings
with "=", so this doesn't confuse the parser.  To preserve
order, bare names after the first assignment are treated
as pseudo-assignments, specially marked as having no value
(which is different from an assignment with an empty value).

The execcmd code then handles WC_TYPESET much like WC_SIMPLE, but if
it's executing a builtin it extracts the assignments, sticks them in a
linked list, and performs prefork and globbing expansion.  I enhanced
struct asgment to do this: it's got an embedded LinkNode, which we do a
lot with HashNode but not elsewhere with LinkNode, however I think it
works OK.

The bin_typeset handler has a different interface from the other
builtins, accepting this extra linked list.  The flag BINF_ASSIGN in the
table indicates this (obviously it's needed for all instances of
bin_typeset, not just those that handle the extra assignments --- which
just get a null list).

There are other tweaks to bin_typeset() and typeset_single() to handle
the new cases.  This is already hairy code, so there could easily be
oddities here, but they don't generally affect things that already work,
so far as I can see.  The key changes are that getasg() is enhanced to
pull values off argv and the new list; when creating a parameter with an
array assignment it's turned into PM_ARRAY; and setsparam() calls now
have parallel setaparam() calls for the other case.

Currently the reserved words are enabled since in theory the disruption
to existing working code should be minimal.  If we find a good reason we
can disable them by default in native mode.

I've got as far as a successful

% typeset array=(one two buckle my shoe)
% print -l $array
one
two
buckle
my
shoe

but there's a lot more debugging and testing to do.  Also, execcmd()
needs some work --- currently, the reserved word doesn't actually force
you to use a builtin of the same name, we just rely on execcmd() finding
the builtin in the hash table.  If that was disabled but there was an
external command of the same name it would execute that, missing the
special assigment commands on the end.  This probably isn't the right
thing to do.

This is on a branch typeset-array, which I'll push so you can have a
look.  When it's stable I'll rebase, squash and merge -ff-only onto the
end of master.

pws

Re: Typeset with array

[Peter Stephenson]

On Fri, 19 Jun 2015 12:39:30 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> I've got as far as a successful
> 
> % typeset array=(one two buckle my shoe)
> % print -l $array
> one
> two
> buckle
> my
> shoe
> 
> but there's a lot more debugging and testing to do.
> 
> This is on a branch typeset-array, which I'll push so you can have a
> look.

Pushed.  Fairly stable, but a couple of tests failing:

- I haven't implemented XTRACE yet.  Will follow.

- The new interface "fixes" the old behaviour

  typeset noktfoo=`echo noktarg1 noktarg2`

so that this now assigns a scalar to noktfoo with no wordsplitting.
This is certainly correct for the interface we are trying to implement,
because it's what bash does.  However, it was previously controlled by
KSH_TYPESET.  What's the right way to do it from now on?  I can probably
kludge in the old behaviour with NO_KSH_TYPESET if it seems desirable,
though logically that shouldn't really control the reserved word
interface which should parse consistently.

Anyway, I haven't touched that test case for nokshtypeset yet.

pws

Re: Typeset with array

[Bart Schaefer]

On Jun 19, 12:39pm, Peter Stephenson wrote:
}
} Building on the above, I've developed a Cunning Plan so that it's not
} too disruptive and is a few days rather than weeks of spare time.
} 
} It works like this.
} 
} Reserved words are added for typeset synonyms that can handle arrays:
} [...]
} The reserved words trigger variant handling in par_simple(): WC_TYPESET
} is very similar to WC_SIMPLE but there's an extra count and set of
} assignments tacked on the end.

Thanks for going on with this.  I had been toying with the idea of having
the reserved word act something like

    typeset -L2 name foo=bar array=(here there)
==> foo=bar array=(here there) builtin typeset -L2 name foo array

so that the parsing of the builtin remained unchanged, it would just need
a flag to say that the temporary values from the prefix assignment should
become the stored values of the named arguments.  This is similar to e.g.

    y=3 integer x=y

which assigns 3 to x.

However, I'm not up to speed enough on wordcode to not waste a lot of
extra time figuring out how to perform that transformation.

The other complications with any implementation are:

1. It's now possible to assign both arrays and scalars in one statement.

 - what happens for  typeset -a x="string" y=(word list)  ??

2. We don't have a distinct syntax for associative array assignment, so
this still works only for ordinary arrays.

 - ksh of course uses  h=([key] value)  to assign an associative array.

Re: Typeset with array

[Oliver Kiddle]

Peter wrote:
> Here's where the Cunning Plan comes in: the spcial assignment behaviour

Firstly, this looks fantastic. Much of my initial basic testing seems to
already be working.

I'll mention some of the things I tried. Just in case there's something
you hadn't thought of. Some combinations aren't especially sane but
might want an error message.

  local -a foo=one
So scope needs to be handled first and array conversion later.
Similarly: typeset -i foo=(23)

I did something like the following in _git recently. It works
  arr=( one two three )
  local $arr
There is also
  local $^arr=foo
and 
  local $^^arr=foo
The nearest in bash would be:
  declare {a,b,c}=foo
which works, but not:
  declare {a,b,c}=(one two)

Bash apparently lets you use += with declare but it seems to be
meaningless because it is not using the value from the outer scope:
  $ foo=hello
  $ function foo {
  > local foo+=bye
  > echo $foo
  > }
  $ foo
  bye
  $ echo $foo
  hello
Ksh doesn't allow it. Printing an error in this case seems best which is
what you have: typeset: not valid in this context: var+

Bash allows array element assignments:
  typeset var[4]=hello
Zsh now prints "can't create local array elements"
Including when not in a function.

That could also be extended to something like var[2,7]=(1 2)

This is perhaps jumping ahead a bit but for completion, it seems we now
get -command- context after typeset. As long as -var- context is applied
inside the actual contexts, having -command- context otherwise for
typeset would probably work fine.

It seems that it is already possible to have assignments after nocorrect
so there may already be handling for this situation by copying whatever
is done for nocorrect.

Is there a reason why noglob can't precede assignments. That could be
useful.

Oliver

Re: Typeset with array

[Bart Schaefer]

On Jun 19,  7:36pm, Oliver Kiddle wrote:
}
} Is there a reason why noglob can't precede assignments. That could be
} useful.

The most obvious reason is that noglob is a precommand modifier, which
quite literally means the parser is looking for a command to follow it
and doesn't find one.

It's the same reason that "exec x=foo" doesn't work, although zsh doesn't
discover that "x=foo" is not a command until the parent shell has already
committed to exiting (in bash, an interactive shell returns to the prompt
on "exec command-that-does-not-exist").

On the other hand nocorrect is allowed to appear before assignments, so
there probably isn't any overriding reason that noglob can't be also,
other than that nocorrect is a reserved word and noglob is not.

Re: Typeset with array

[Bart Schaefer]

On Jun 19,  3:06pm, Peter Stephenson wrote:
}
} - The new interface "fixes" the old behaviour
} 
}   typeset noktfoo=`echo noktarg1 noktarg2`
} 
} so that this now assigns a scalar to noktfoo with no wordsplitting.
} This is certainly correct for the interface we are trying to implement,
} because it's what bash does.  However, it was previously controlled by
} KSH_TYPESET.  What's the right way to do it from now on?

Does this raise a larger question of whether the reserved word should be
enabled by default in "zsh mode"?

If this only affects the reserved word interface and the reserved word
has to be explicitly enabled (by "emulate" or "enable") then there's no
issue.

OTOH it would be nice to have the reserved word on by default.  Using
array assignment syntax in the arguments of typeset would generally have
been a syntax or globbing error in the past, so this word splitting is
the primary place where a valid statement might change its semantics.

} kludge in the old behaviour with NO_KSH_TYPESET if it seems desirable,

Are there any other practical differences between KSH_TYPESET and having
the reserved word enabled?

} though logically that shouldn't really control the reserved word
} interface which should parse consistently.

Agreed.

Re: Typeset with array

[Peter Stephenson]

On Fri, 19 Jun 2015 11:54:54 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Does this raise a larger question of whether the reserved word should be
> enabled by default in "zsh mode"?

Yes, indeed.

> If this only affects the reserved word interface and the reserved word
> has to be explicitly enabled (by "emulate" or "enable") then there's no
> issue.
> 
> OTOH it would be nice to have the reserved word on by default.  Using
> array assignment syntax in the arguments of typeset would generally have
> been a syntax or globbing error in the past, so this word splitting is
> the primary place where a valid statement might change its semantics.

Right.  I'm assuming the ability to do

  local scalar=stuff array=(more stuff)

is so obviously natural we should find some way of enabling it by
default unless we hit some insuperable objection.

> } kludge in the old behaviour with NO_KSH_TYPESET if it seems desirable,
> 
> Are there any other practical differences between KSH_TYPESET and having
> the reserved word enabled?

Effectively, KSH_TYPESET was just a partial fix-up for the reserved word
behaviour, missing array handling, Actually, more than that, in bash at
least (I wouldn't be surprised if ksh was similar),

$ declare=declare
$ $declare foo=`echo bar one=two`
$ echo $one
two

which is NO_KSH_TYPESET behavioru --- so logically KSH_TYPESET should be
*off* from now on, but the reserved word scheme on, in emulation mode,
whatever happens in native mode.

> } though logically that shouldn't really control the reserved word
> } interface which should parse consistently.
> 
> Agreed.

I think there are two arguments around NO_KSH_TYPESET behaviour in
native mode (however arrived at):

1. We've always done it that way so should continue.

2. The only reason we left it that way by default is the fact that
KSH_TYPESET was only ever a partial fix that didn't parse assignments
properly.  With that objection removed there's no real reason for
NO_KSH_TYPESET; and further anyone relying on

   typeset foo=`echo one=two`

doing what it now does needs to get out more as it's an accident
waiting to happen.

I'm inclining to 2, but that may be because (as with the case parsing
change) I'm mssing some more widespread effect.  I'd vaguely guess that
anyone that didn't read the manual in some detail wouldn't be aware of
the current state of affairs.

The various other questions noted by you and Oliver are less fraught.

pws

Re: Typeset with array

[Peter Stephenson]

On Fri, 19 Jun 2015 09:09:04 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> 1. It's now possible to assign both arrays and scalars in one statement.
> 
>  - what happens for  typeset -a x="string" y=(word list)  ??

Oliver noted this, too (most of his examples should in principle do
something sensible eventually but I haven't got round to that level of
detail yet).  I think it doesn't matter that much if either this gets
silently turned into x=("string"), or if there's an error on the x
assignment, as long as we pick one and document it.

> 2. We don't have a distinct syntax for associative array assignment, so
> this still works only for ordinary arrays.
> 
>  - ksh of course uses  h=([key] value)  to assign an associative array.

Well, we don't have that syntax, but this should work:

  typeset -A hash=(bob robert bill william)

but doesn't because of a bug fixed below.  If you're playing along at
home --- this is exclusive to the typeset-array branch.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index b34669f..6cccf53 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1998,7 +1998,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 
     /* attempting a type conversion, or making a tied colonarray? */
     tc = 0;
-    if (ASG_ARRAYP(asg))
+    if (ASG_ARRAYP(asg) && PM_TYPE(on) == PM_SCALAR)
 	on |= PM_ARRAY;
     if (usepm && ASG_ARRAYP(asg) && newspecial == NS_NONE &&
 	PM_TYPE(pm->node.flags) != PM_ARRAY &&

Re: Typeset with array

[Eric Cook]

On 06/19/2015 02:54 PM, Bart Schaefer wrote:
>
> Does this raise a larger question of whether the reserved word should be
> enabled by default in "zsh mode"?
>
>
I personally like the consistency of builtin commands being parsed like
normal commands.
weird stuff like:
http://lists.gnu.org/archive/html/bug-bash/2014-12/msg00028.html
Don't happen that way.

Re: Typeset with array

[Bart Schaefer]

On Jun 19,  9:16pm, Peter Stephenson wrote:
}
} 2. The only reason we left it that way by default is the fact that
} KSH_TYPESET was only ever a partial fix that didn't parse assignments
} properly.  With that objection removed there's no real reason for
} NO_KSH_TYPESET; and further anyone relying on
} 
}    typeset foo=`echo one=two`
} 
} doing what it now does needs to get out more as it's an accident
} waiting to happen.

There are a few instances of declarations using foo=`command` and/or
foo=$(command) in the completion code.  Arbitrary examples:

    _tla:47:  local expl1 expl2 tree_version=`$TLA tree-version`
    _rebootin:5:local loader=$(sudo detectloader -q)

Probably these are expecting single word responses.

In any case I also lean to this answer.

On Jun 19,  9:32pm, Peter Stephenson wrote:
}
} > 2. We don't have a distinct syntax for associative array assignment, so
} > this still works only for ordinary arrays.
} 
} Well, we don't have [ksh] syntax, but this should work:
} 
}   typeset -A hash=(bob robert bill william)

That's not quite what I was getting at -- rather I was pointing out that
you can't do

    typeset scalar=string array=(elem list) assoc=(key value)

all in one statement, because there's no way to distinguish (key value)
from (elem list) by inspection.

Re: Typeset with array

[Peter Stephenson]

On Fri, 19 Jun 2015 20:50:01 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> There are a few instances of declarations using foo=`command` and/or
> foo=$(command) in the completion code.  Arbitrary examples:
> 
>     _tla:47:  local expl1 expl2 tree_version=`$TLA tree-version`
>     _rebootin:5:local loader=$(sudo detectloader -q)
> 
> Probably these are expecting single word responses.

Yes, this looks to me like hard evidence (as opposed to personal
prejudice) in favour of the chnage.

pws

Re: Typeset with array

[Peter Stephenson]

On Fri, 19 Jun 2015 12:39:30 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Also, execcmd() needs some work --- currently, the reserved word
> doesn't actually force you to use a builtin of the same name, we just
> rely on execcmd() finding the builtin in the hash table.  If that was
> disabled but there was an external command of the same name it would
> execute that, missing the special assigment commands on the end.  This
> probably isn't the right thing to do.

This fixes this: we look up the same handler as the builtin for the
reserved word, but ignore whether the builtin is disabled because we
know the reserved word is enabled.  We now force the WC_TYPESET
interface even if there aren't any assignments to ensure consistency.

"builtin typeset" used the builtin interface even if the reserved word
is enabled --- I think that's correct.

One drive-by fix to ensure we don't force an array where there's an
existing parameter we can use that's a hash.

With a few more tests I think this is just about ready for the master
branch.

pws


diff --git a/Src/builtin.c b/Src/builtin.c
index 5eb7bfb..dd28c8b 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2020,7 +2020,8 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 
     /* attempting a type conversion, or making a tied colonarray? */
     tc = 0;
-    if (ASG_ARRAYP(asg) && PM_TYPE(on) == PM_SCALAR)
+    if (ASG_ARRAYP(asg) && PM_TYPE(on) == PM_SCALAR &&
+	!(usepm && (PM_TYPE(pm->node.flags) & (PM_ARRAY|PM_HASHED))))
 	on |= PM_ARRAY;
     if (usepm && ASG_ARRAYP(asg) && newspecial == NS_NONE &&
 	PM_TYPE(pm->node.flags) != PM_ARRAY &&
diff --git a/Src/exec.c b/Src/exec.c
index 6066d55..57e8f63 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2543,14 +2543,26 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    checked = !has_token(cmdarg);
 	    if (!checked)
 		break;
-	    if (!(cflags & (BINF_BUILTIN | BINF_COMMAND)) &&
-		(hn = shfunctab->getnode(shfunctab, cmdarg))) {
-		is_shfunc = 1;
-		break;
-	    }
-	    if (!(hn = builtintab->getnode(builtintab, cmdarg))) {
-		checked = !(cflags & BINF_BUILTIN);
-		break;
+	    if (type == WC_TYPESET &&
+		(hn = builtintab->getnode2(builtintab, cmdarg))) {
+		/*
+		 * If reserved word for typeset command found (and so
+		 * enabled), use regardless of whether builtin is
+		 * enabled as we share the implementation.
+		 *
+		 * Reserved words take precedence over shell functions.
+		 */
+		checked = 1;
+	    } else {
+		if (!(cflags & (BINF_BUILTIN | BINF_COMMAND)) &&
+		    (hn = shfunctab->getnode(shfunctab, cmdarg))) {
+		    is_shfunc = 1;
+		    break;
+		}
+		if (!(hn = builtintab->getnode(builtintab, cmdarg))) {
+		    checked = !(cflags & BINF_BUILTIN);
+		    break;
+		}
 	    }
 	    orig_cflags |= cflags;
 	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
diff --git a/Src/parse.c b/Src/parse.c
index a95ec60..5357851 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -131,13 +131,11 @@ struct heredocs *hdocs;
  *     - followed by strings
  *
  *   WC_TYPESET
- *     Variant of WC_SIMPLE used when trailing assignments are
- *     needed.  N.B.: if they are not, we use WC_SIMPLE even
- *     if this is a TYPESET keyword.
+ *     Variant of WC_SIMPLE used when TYPESET reserved word found.
  *     - data contains the number of string arguments (plus command)
  *     - followed by strings
  *     - followed by number of assignments
- *     - followed by assignments
+ *     - followed by assignments if non-zero number.
  *
  *   WC_SUBSH
  *     - data unused
@@ -1728,7 +1726,7 @@ static int
 par_simple(int *cmplx, int nr)
 {
     int oecused = ecused, isnull = 1, r, argc = 0, p, isfunc = 0, sr = 0;
-    int c = *cmplx, nrediradd, assignments = 0, ppost = 0;
+    int c = *cmplx, nrediradd, assignments = 0, ppost = 0, is_typeset = 0;
     wordcode postassigns = 0;
 
     r = ecused;
@@ -1814,7 +1812,7 @@ par_simple(int *cmplx, int nr)
 	    incmdpos = 0;
 
 	    if (tok == TYPESET)
-		intypeset = 1;
+		intypeset = is_typeset = 1;
 
 	    if (!isset(IGNOREBRACES) && *tokstr == Inbrace)
 	    {
@@ -2024,9 +2022,12 @@ par_simple(int *cmplx, int nr)
     intypeset = 0;
 
     if (!isfunc) {
-	if (postassigns) {
+	if (is_typeset) {
 	    ecbuf[p] = WCB_TYPESET(argc);
-	    ecbuf[ppost] = postassigns;
+	    if (postassigns)
+		ecbuf[ppost] = postassigns;
+	    else
+		ecadd(0);
 	} else
 	    ecbuf[p] = WCB_SIMPLE(argc);
     }

Re: Typeset with array

[Peter Stephenson]

On Sun, 21 Jun 2015 21:05:12 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> With a few more tests I think this is just about ready for the master
> branch.

(And some documentation.)  Here are some very simple tests.

I hadn't relaised typeset -L and -R don't work with arrays, but
apparently that's not new.  It doesn't appear to be documented.

pws

diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 1819b6c..75c475c 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -508,3 +508,20 @@
 >a2=(three four)
 >typeset -r r1=yes
 >typeset -r r2=no
+
+  one=hidden two=hidden three=hidden four=hidden five=hidden
+  fn() {
+     local bleugh="four=vier"
+     typeset -R10 one=eins two=(zwei dio) three $bleugh five=(cinq cinque)
+     three=drei
+     print -l $one $two $three $four $five
+  }
+  fn
+0:typeset reserved word interface: basic
+>      eins
+>zwei
+>dio
+>      drei
+>      vier
+>cinq
+>cinque

Re: Typeset with array

[Peter Stephenson]

Some more tests and one quite subtle fix.

I think this is now basically working.  Any more comments, or should I
roll this out and see what happens?

pws

diff --git a/Src/parse.c b/Src/parse.c
index 5357851..477f8a0 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1898,10 +1898,18 @@ par_simple(int *cmplx, int nr)
 	    parr = ecadd(0);
 	    ecstr(tokstr);
 	    cmdpush(CS_ARRAY);
+	    /*
+	     * Careful here: this must be the typeset case,
+	     * but we need to tell the lexer not to look
+	     * for assignments until we've finished the
+	     * present one.
+	     */
+	    intypeset = 0;
 	    zshlex();
 	    n = par_nl_wordlist();
 	    ecbuf[parr] = WCB_ASSIGN(WC_ASSIGN_ARRAY, WC_ASSIGN_NEW, n);
 	    cmdpop();
+	    intypeset = 1;
 	    if (tok != OUTPAR)
 		YYERROR(oecused);
 	    zshlex();
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 75c475c..e5c4310 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -22,6 +22,8 @@
 
 %prep
 
+  mkdir typeset.tmp && cd typeset.tmp
+
   setopt noglob
 
   scalar=scalar
@@ -517,6 +519,7 @@
      print -l $one $two $three $four $five
   }
   fn
+  print -l $one $two $three $four $five
 0:typeset reserved word interface: basic
 >      eins
 >zwei
@@ -525,3 +528,82 @@
 >      vier
 >cinq
 >cinque
+>hidden
+>hidden
+>hidden
+>hidden
+>hidden
+
+  (
+  setopt glob
+  mkdir -p arrayglob
+  touch arrayglob/{one,two,three,four,five,six,seven}
+  fn() {
+    typeset array=(arrayglob/[tf]*)
+    print -l ${array:t}
+    #
+    typeset {first,second,third}=the_same_value array=(
+    extends
+    over
+    multiple
+    lines
+    )
+    print -l $first $second $third "$array"
+    #
+    integer i=$(echo 1 + 2 + 3 + 4)
+    print $i
+    #
+    # only noted by accident this was broken..
+    # we need to turn off special recognition
+    # of assignments within assignments...
+    typeset careful=( i=1 j=2 k=3 )
+    print -l $careful
+  }
+  fn
+  )
+0:typeset reserved word, more complicated cases
+>five
+>four
+>three
+>two
+>the_same_value
+>the_same_value
+>the_same_value
+>extends over multiple lines
+>10
+>i=1
+>j=2
+>k=3
+
+  (
+     # reserved word is recognised at parsing.
+     # yes, this is documented.
+     # anyway, that means we need to
+     # re-eval the function...
+     fn='
+     fn() {
+        typeset foo=`echo one word=two`
+        print $foo
+        print $word
+     }
+     '
+     print reserved
+     eval $fn; fn
+     print builtin
+     disable -r typeset
+     eval $fn; fn
+     enable -r typeset
+     disable typeset
+     print reserved
+     eval $fn;fn
+  )
+0:reserved word and builtin interfaces
+>reserved
+>one word=two
+>
+>builtin
+>one
+>two
+>reserved
+>one word=two
+>

Re: Typeset with array

[Mikael Magnusson]

On Tue, Jun 23, 2015 at 6:47 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> Some more tests and one quite subtle fix.
>
> I think this is now basically working.  Any more comments, or should I
> roll this out and see what happens?

I tried it and didn't encounter any problems so far (amazing, right?).

This isn't really related to this series, but I noticed it now for the
first time; what's the deal with the space at the end of associative
arrays in typeset -p output?
% typeset -a a=(b); typeset -A b=(b c); typeset -p a b
typeset -a a
a=(b)
typeset -A b
b=(b c )

-- 
Mikael Magnusson

Re: Typeset with array

[Bart Schaefer]

On Jun 23,  7:52pm, Mikael Magnusson wrote:
}
} This isn't really related to this series, but I noticed it now for the
} first time; what's the deal with the space at the end of associative
} arrays in typeset -p output?

With a normal array, the number of elements is known, so you know when
you've reached the last one and can skip printing the space after it.

When traversing a hash table, you don't know if you've reached the last
element until you've passed it; it's more straighforward to print out
("%s %s ", key, value) for every element than to keep track of whether
you are past the first element but not yet at the last one.

If I were going to "fix" it for no reason other than the consistency,
I'd simply add MORE spaces rather than try to squash that one.

typeset -a a
a=( b )
typeset -A b
b=( b c )

Re: Typeset with array

[Peter Stephenson]

On Tue, 23 Jun 2015 13:17:06 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> If I were going to "fix" it for no reason other than the consistency,
> I'd simply add MORE spaces rather than try to squash that one.
> 
> typeset -a a
> a=( b )
> typeset -A b
> b=( b c )

That's how XTRACE output works, so it would make sense.

pws

Re: Typeset with array

[Mikael Magnusson]

On Tue, Jun 23, 2015 at 10:17 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Jun 23,  7:52pm, Mikael Magnusson wrote:
> }
> } This isn't really related to this series, but I noticed it now for the
> } first time; what's the deal with the space at the end of associative
> } arrays in typeset -p output?
>
> With a normal array, the number of elements is known, so you know when
> you've reached the last one and can skip printing the space after it.
>
> When traversing a hash table, you don't know if you've reached the last
> element until you've passed it; it's more straighforward to print out
> ("%s %s ", key, value) for every element than to keep track of whether
> you are past the first element but not yet at the last one.

Ah, that makes sense.

> If I were going to "fix" it for no reason other than the consistency,
> I'd simply add MORE spaces rather than try to squash that one.
>
> typeset -a a
> a=( b )
> typeset -A b
> b=( b c )

This would also be consistent with xtrace output, so I agree :).

-- 
Mikael Magnusson

Re: Typeset with array

[Bart Schaefer]

On Jun 23,  5:47pm, Peter Stephenson wrote:
}
} I think this is now basically working.  Any more comments, or should I
} roll this out and see what happens?

I'd say go for it.

} +	    /*
} +	     * Careful here: this must be the typeset case,
} +	     * but we need to tell the lexer not to look
} +	     * for assignments until we've finished the
} +	     * present one.
} +	     */

This has me trying to think of ways to implement the ksh ([key]=value)
syntax.

    typeset -a varname=([k1]=v1 [k2]=v2)

is just

    typeset -A varname varname[k1]=v1 varname[k2]=v2

There's probably a not-too-horrible way to do that if already looking
for assignments inside the parens ...

But I wouldn't suggest delaying any further for that.

typeset -p with assoc array (was Re: Typeset with array)

[Bart Schaefer]

On Jun 23, 10:24pm, Mikael Magnusson wrote:
} 
} > If I were going to "fix" it for no reason other than the consistency,
} > I'd simply add MORE spaces rather than try to squash that one.
} 
} This would also be consistent with xtrace output, so I agree :).

This is not difficult, and doesn't conflict with the typeset-array
branch as far as I can see, so ... patch below.

However, it does point out that the typeset-array branch could update
printparamnode() to emit

    typeset -a a=( b )

instead of the historic two-line format.  (Urk, it'd be kind of ugly
to make that depend on whether the reserved word was enabled.)


diff --git a/Src/params.c b/Src/params.c
index 3b75735..f8d039a 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -5091,8 +5091,10 @@ printparamvalue(Param p, int printflags)
 	break;
     case PM_ARRAY:
 	/* array */
-	if (!(printflags & PRINT_KV_PAIR))
+	if (!(printflags & PRINT_KV_PAIR)) {
 	    putchar('(');
+	    putchar(' ');
+	}
 	u = p->gsu.a->getfn(p);
 	if(*u) {
 	    quotedzputs(*u++, stdout);
@@ -5101,13 +5103,17 @@ printparamvalue(Param p, int printflags)
 		quotedzputs(*u++, stdout);
 	    }
 	}
-	if (!(printflags & PRINT_KV_PAIR))
+	if (!(printflags & PRINT_KV_PAIR)) {
+	    putchar(' ');
 	    putchar(')');
+	}
 	break;
     case PM_HASHED:
 	/* association */
-	if (!(printflags & PRINT_KV_PAIR))
+	if (!(printflags & PRINT_KV_PAIR)) {
 	    putchar('(');
+	    putchar(' ');
+	}
 	{
             HashTable ht = p->gsu.h->getfn(p);
             if (ht)

Re: typeset -p with assoc array (was Re: Typeset with array)

[Bart Schaefer]

Here are Test/* changes to go with workers/35581 in case we want it.


diff --git a/Test/A06assign.ztst b/Test/A06assign.ztst
index a4401cb..302659c 100644
--- a/Test/A06assign.ztst
+++ b/Test/A06assign.ztst
@@ -430,7 +430,7 @@
 0:GLOB_ASSIGN with numeric types
 >typeset -i i=0
 >typeset -a n
->n=(tmpfile1 tmpfile2)
+>n=( tmpfile1 tmpfile2 )
 >typeset x=tmpfile2
 >typeset -E f=4.000000000e+00
 
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 57a7caa..057f74b 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -451,7 +451,7 @@
  fn
 1:declare -p shouldn't create scoped values
 >typeset -a array
->array=(foo bar)
+>array=( foo bar )
 ?fn:typeset: no such variable: nonexistent
 
  unsetopt typesetsilent
@@ -502,9 +502,9 @@
   typeset -pm 'r[12]'
 0:readonly -p output
 >typeset -a a1
->a1=(one two)
+>a1=( one two )
 >typeset -ar a1
 >typeset -a a2
->a2=(three four)
+>a2=( three four )
 >typeset -r r1=yes
 >typeset -r r2=no

Re: Typeset with array

[Peter Stephenson]

On Tue, 23 Jun 2015 13:25:22 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jun 23,  5:47pm, Peter Stephenson wrote:
> }
> } I think this is now basically working.  Any more comments, or should I
> } roll this out and see what happens?
> 
> I'd say go for it.

OK, here's one more test that's been bugging me (but does pass), then
I'll rebase onto the end of master and do any remaining tweaks there.

> 
> } +	    /*
> } +	     * Careful here: this must be the typeset case,
> } +	     * but we need to tell the lexer not to look
> } +	     * for assignments until we've finished the
> } +	     * present one.
> } +	     */
>
> This has me trying to think of ways to implement the ksh ([key]=value)
> syntax.
> 
>     typeset -a varname=([k1]=v1 [k2]=v2)

One possibility is to hook into ENVARRAY handling in parse.c at two
places. par_nl_worldist() is a key to this, though there's one
irrelevant case which is arguments for "for".  I don't know if the
answer is to get the lexer to detect k1 and v1 as tokens or to get the
parser to see if it fits that form a word at a time.  Then it needs to
go into wordcode in a special form --- though a specially tagged list of
an even number of items is good enough for this.

However, You could get away with detecting the form until the list is
expanded: that's now in two different places, addvars and execcmd, but
could easily be made common --- put something in front of the
ecgetlist() that retrieves the array.  As long as you detect it before
attempting to glob, to avoid NO_MATCH behaviour, it ought to work.  This
is easier as there are no wordcode changes and I can't see any obvious
gotchas.  I'm not sure what expansions apply: do k* get expanded at all?
Presumably v* get single word expansion?

pws

diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 48d1653..4afb189 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -595,7 +595,7 @@
      enable -r typeset
      disable typeset
      print reserved
-     eval $fn;fn
+     eval $fn; fn
   )
 0:reserved word and builtin interfaces
 >reserved
@@ -607,3 +607,47 @@
 >reserved
 >one word=two
 >
+
+  fn() {
+    emulate -L zsh
+    setopt typeset_silent
+    local k
+    typeset -A hash=(k1 v1 k2 v2)
+    typeset foo=word array=(more than one word)
+    for k in ${(ko)hash}; do
+      print $k $hash[$k]
+    done
+    print -l $foo $array
+    typeset -A hash
+    typeset foo array
+    for k in ${(ko)hash}; do
+      print $k $hash[$k]
+    done
+    print -l $foo $array
+    typeset hash=(k3 v3 k4 v4) array=(odd number here)
+    for k in ${(ko)hash}; do
+      print $k $hash[$k]
+    done
+    print -l $array
+  }
+  fn
+0:typeset preserves existing variable types
+>k1 v1
+>k2 v2
+>word
+>more
+>than
+>one
+>word
+>k1 v1
+>k2 v2
+>word
+>more
+>than
+>one
+>word
+>k3 v3
+>k4 v4
+>odd
+>number
+>here

Re: Typeset with array

[Peter Stephenson]

On Wed, 24 Jun 2015 10:14:04 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Tue, 23 Jun 2015 13:25:22 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Jun 23,  5:47pm, Peter Stephenson wrote:
> > } I think this is now basically working.  Any more comments, or should I
> > } roll this out and see what happens?
> > 
> > I'd say go for it.
> 
> OK, here's one more test that's been bugging me (but does pass), then
> I'll rebase onto the end of master and do any remaining tweaks there.

Pushed 39b28980f3.  I'll leave typeset-array for historical interest.

I presume this is going to have at least *some* knock on effects...

pws

Re: Typeset with array

[Roman Neuhauser]

# p.stephenson@samsung.com / 2015-06-24 10:29:42 +0100:
> On Wed, 24 Jun 2015 10:14:04 +0100
> Peter Stephenson <p.stephenson@samsung.com> wrote:
> > On Tue, 23 Jun 2015 13:25:22 -0700
> > Bart Schaefer <schaefer@brasslantern.com> wrote:
> > > On Jun 23,  5:47pm, Peter Stephenson wrote:
> > > } I think this is now basically working.  Any more comments, or should I
> > > } roll this out and see what happens?
> > > 
> > > I'd say go for it.
> > 
> > OK, here's one more test that's been bugging me (but does pass), then
> > I'll rebase onto the end of master and do any remaining tweaks there.
> 
> Pushed 39b28980f3.

Woot! Yay! Thanks!

-- 
roman

Re: Typeset with array

[Mikael Magnusson]

On Wed, Jun 24, 2015 at 11:29 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 24 Jun 2015 10:14:04 +0100
> Peter Stephenson <p.stephenson@samsung.com> wrote:
>> On Tue, 23 Jun 2015 13:25:22 -0700
>> Bart Schaefer <schaefer@brasslantern.com> wrote:
>> > On Jun 23,  5:47pm, Peter Stephenson wrote:
>> > } I think this is now basically working.  Any more comments, or should I
>> > } roll this out and see what happens?
>> >
>> > I'd say go for it.
>>
>> OK, here's one more test that's been bugging me (but does pass), then
>> I'll rebase onto the end of master and do any remaining tweaks there.
>
> Pushed 39b28980f3.  I'll leave typeset-array for historical interest.
>
> I presume this is going to have at least *some* knock on effects...

Now that it's on master, my curse kicked in and I found a problem,
% foo() { typeset -g foo=bar }; which foo
foo () {
    typeset -gfoo=bar
}

Notice the lack of space after -g.

-- 
Mikael Magnusson

Re: Typeset with array

[Peter Stephenson]

On Wed, 24 Jun 2015 15:00:22 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> % foo() { typeset -g foo=bar }; which foo
> foo () {
>     typeset -gfoo=bar
> }
> 
> Notice the lack of space after -g.

I've just remembered what I meant to test.

pws

diff --git a/Src/text.c b/Src/text.c
index a72ab33..3287c54 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -189,6 +189,8 @@ taddassign(wordcode code, Estate state, int typeset)
 static void
 taddassignlist(Estate state, wordcode count)
 {
+    if (count)
+	taddchr(' ');
     while (count--) {
 	wordcode code = *state->pc++;
 	taddassign(code, state, 1);
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 4afb189..e6285bc 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -651,3 +651,15 @@
 >odd
 >number
 >here
+
+  fn() { typeset foo bar thing=this stuff=(that other) more=woevva; }
+  which -x2 fn
+  fn2() { typeset assignfirst=(why not); }
+  which -x2 fn2
+0:text output from typeset
+>fn () {
+>  typeset foo bar thing=this stuff=(that other) more=woevva 
+>}
+>fn2 () {
+>  typeset assignfirst=(why not) 
+>}

Re: Typeset with array

[Bart Schaefer]

On Jun 24, 10:14am, Peter Stephenson wrote:
}
} > This has me trying to think of ways to implement the ksh ([key]=value)
} > syntax.
} > 
} >     typeset -a varname=([k1]=v1 [k2]=v2)

Incidentally, I just stumbled over the bash feature that you can set
elements of a regular array this way as well.

bash$ array=([0]="a" [2]="b" [1]="")
bash$ typeset -p array
declare -a array='([0]="a" [1]="" [2]="b")'
bash$ 

I believe someone else already pointed out that looking inside the quoted
value for a parenthesized list has its own set of problems.

Ksh always creates an associative array with that syntax unless explictly
told otherwise, but it accepts it for normal arrays:

ksh$ array=([0]="a" [2]="b" [1]="")
ksh$ typeset -p array
typeset -A array=([0]=a [1]='' [2]=b)
ksh$ typeset -a array=([0]="a" [2]="b" [1]="")
ksh$ typeset -p array
typeset -a array=(a '' b)
ksh$ 

So it really is just the same as mapping the variable name across the
values in the list.

} However, You could get away with detecting the form until the list is

"not detecting"?

} expanded: that's now in two different places, addvars and execcmd, but
} could easily be made common --- put something in front of the
} ecgetlist() that retrieves the array.  As long as you detect it before
} attempting to glob, to avoid NO_MATCH behaviour, it ought to work.  This
} is easier as there are no wordcode changes and I can't see any obvious
} gotchas.  I'm not sure what expansions apply: do k* get expanded at all?
} Presumably v* get single word expansion?

ksh$ echo D*
Doc
ksh$ typeset array=([D*]=D*)
ksh$ typeset -p array
typeset -A array=(['D*']='D*')
ksh$ 

But note:

ksh$ typeset -A array=([*]=*)
ksh: *: invalid subscript in assignment
ksh$ 

Re: Typeset with array

[Peter Stephenson]

On Fri, 19 Jun 2015 19:36:26 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>   local -a foo=one
> So scope needs to be handled first and array conversion later.
> Similarly: typeset -i foo=(23)

I decided to make these errors --- the user is in a good position to do
the right thing here since it's staring them in the face and fixing
things up in typeset by guessing what the user actually meant is (you
can trust me on this :-/) very messy, so I think this is reasonable.

> I did something like the following in _git recently. It works
>   arr=( one two three )
>   local $arr
> There is also
>   local $^arr=foo
> and 
>   local $^^arr=foo
> The nearest in bash would be:
>   declare {a,b,c}=foo
> which works, but not:
>   declare {a,b,c}=(one two)

I think this is now working as expected and documented.

> Bash apparently lets you use += with declare but it seems to be
> meaningless because it is not using the value from the outer scope:
>   $ foo=hello
>   $ function foo {
>   > local foo+=bye
>   > echo $foo
>   > }
>   $ foo
>   bye
>   $ echo $foo
>   hello
> Ksh doesn't allow it. Printing an error in this case seems best which is
> what you have: typeset: not valid in this context: var+

This is still the case.

> Bash allows array element assignments:
>   typeset var[4]=hello
> Zsh now prints "can't create local array elements"
> Including when not in a function.

The problem isn't the assignment but you haven't yet told the shell that
"var" is a (local?) variable or what type it is.  So it's claiming to be
a bit confused. Do you really mean it to update an existing array var,
as it looks like, or don't you?  Or are you replacing the 4th character
of a scalar (which doesn't actually exist yet) with a string?  Both
work if var already exists, but if it doesn't it's syntactically
ambiguous even if you could probably guess that it should create ('' ''
'' hello).  We could tweak this, but it's at the point where I'm not
100% convinced it's the right thing to do.

> That could also be extended to something like var[2,7]=(1 2)

Extending assignments (at least to existing arrays) to handle array
slices would certainly be natural, yes.  I hope must of the mechanism is
already present and just needs borrowing from addvars().

> This is perhaps jumping ahead a bit but for completion, it seems we now
> get -command- context after typeset. As long as -var- context is applied
> inside the actual contexts, having -command- context otherwise for
> typeset would probably work fine.

That still needs looking at --- as this is a rather grungy interface
into the main shell it's a bit of a dark art whether this should be
tweaked by fixing up the context or fixing up the handler, or whether
it's one of the cases where the Red Death holds illimitable sway and
dominion.  With any luck one of the first two is possible.

Scalar and array contexts after typeset assignment seem to be working OK
as a -value-.

pws

Re: Typeset with array

[Bart Schaefer]

On Jun 25, 10:29am, Peter Stephenson wrote:
}
} > Bash allows array element assignments:
} >   typeset var[4]=hello
} > Zsh now prints "can't create local array elements"
} > Including when not in a function.
} 
} The problem isn't the assignment but you haven't yet told the shell that
} "var" is a (local?) variable or what type it is.

Some quick tests:

torch% typeset var var[2]=two 
torch% typeset -p var
typeset var=two
torch% typeset -a array array[2]=two
typeset: array[2]: inconsistent type for assignment
torch% typeset -p array
typeset -a array
array=()
torch% typeset newarray=() newarray[2]=two 
zsh: segmentation fault (core dumped)  Src/zsh -f

#0  0x080bce65 in prefork (list=0x0, flags=2) at ../../zsh-5.0/Src/subst.c:58
#1  0x0806ab0f in execcmd (state=0xbff08f60, input=0, output=0, how=18, 
    last1=2) at ../../zsh-5.0/Src/exec.c:3586
#2  0x08065b69 in execpline2 (state=0xbff08f60, pcode=131, how=18, input=0, 
    output=0, last1=0) at ../../zsh-5.0/Src/exec.c:1718
#3  0x08064f08 in execpline (state=0xbff08f60, slcode=9218, how=18, last1=0)
    at ../../zsh-5.0/Src/exec.c:1501
#4  0x080647a8 in execlist (state=0xbff08f60, dont_change_job=0, exiting=0)
    at ../../zsh-5.0/Src/exec.c:1277
#5  0x0806418e in execode (p=0xb7d68798, dont_change_job=0, exiting=0, 
    context=0x8149bb3 "toplevel") at ../../zsh-5.0/Src/exec.c:1074
#6  0x08080819 in loop (toplevel=1, justonce=0) at ../../zsh-5.0/Src/init.c:207
#7  0x08083c48 in zsh_main (argc=2, argv=0xbff090b4)
    at ../../zsh-5.0/Src/init.c:1675
#8  0x0804c2ea in main (argc=2, argv=0xbff090b4) at ../../zsh-5.0/Src/main.c:93

Re: Typeset with array

[Peter Stephenson]

On Thu, 25 Jun 2015 08:16:53 -0700
> Some quick tests:
> 
> torch% typeset var var[2]=two 
> torch% typeset -p var
> typeset var=two

That's correct: var is created then manipulated as a scalar.  Apart from
localness this is the same effect as

var=
var[2]=two

> torch% typeset -a array array[2]=two
> typeset: array[2]: inconsistent type for assignment
> torch% typeset -p array
> typeset -a array
> array=()

That's also correct.  typeset -a creates arrays, so "array" is
successfully created; however, array[2] is a scalar with a scalar
assignment, so that's an error with the "-a".  (However, if you think
you can work out how to relax the test to allow array elements without
the whole thing blowing up in your face, be my guest...)

> torch% typeset newarray=() newarray[2]=two 

That's the right way to mix arrays and scalars...

> zsh: segmentation fault (core dumped)  Src/zsh -f

..so, as it happens, that's not correct.  That reminds me of something
else I meant to test...

diff --git a/Src/exec.c b/Src/exec.c
index 57e8f63..50a11eb 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3583,15 +3583,18 @@ execcmd(Estate state, int input, int output, int how, int last1)
 			    asg->value.array =
 				ecgetlist(state, WC_ASSIGN_NUM(ac),
 					  EC_DUPTOK, &htok);
-			    prefork(asg->value.array, PREFORK_ASSIGN);
-			    if (errflag) {
-				state->pc = opc;
-				break;
-			    }
-			    globlist(asg->value.array, 0);
-			    if (errflag) {
-				state->pc = opc;
-				break;
+			    if (asg->value.array)
+			    {
+				prefork(asg->value.array, PREFORK_ASSIGN);
+				if (errflag) {
+				    state->pc = opc;
+				    break;
+				}
+				globlist(asg->value.array, 0);
+				if (errflag) {
+				    state->pc = opc;
+				    break;
+				}
 			    }
 			}
 
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index e6285bc..1548b81 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -663,3 +663,16 @@
 >fn2 () {
 >  typeset assignfirst=(why not) 
 >}
+
+  fn() {
+    typeset array=()
+    print ${(t)array} ${#array}
+    typeset gnothergarray=() gnothergarray[1]=yes gnothergarray[2]=no
+    print -l ${(t)gnothergarray} $gnothergarray
+  }
+  fn
+0:can set empty array
+>array-local 0
+>array-local
+>yes
+>no

Re: Typeset with array

[Bart Schaefer]

On Jun 25,  4:39pm, Peter Stephenson wrote:
} Subject: Re: Typeset with array
}
} > torch% typeset -a array array[2]=two
} > typeset: array[2]: inconsistent type for assignment
} > torch% typeset -p array
} > typeset -a array
} > array=()
} 
} That's also correct.  typeset -a creates arrays, so "array" is
} successfully created; however, array[2] is a scalar with a scalar
} assignment, so that's an error with the "-a".

No disagreement, but then:

torch% typeset var=abcd var[2,3]=23
torch% typeset -p var
typeset var=a23d
torch% typeset -a array array[2]=(two)
typeset: array[2]: array elements must be scalar

So slices of scalars work, but not slices of arrays.

Re: Typeset with array

[Peter Stephenson]

On Thu, 25 Jun 2015 09:08:58 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> So slices of scalars work, but not slices of arrays.

Yes, that's what I just told Oliver.

pws

PATCH: array slice

[Peter Stephenson]

On Thu, 25 Jun 2015 10:29:23 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> > That could also be extended to something like var[2,7]=(1 2)
> 
> Extending assignments (at least to existing arrays) to handle array
> slices would certainly be natural, yes.  I hope m[o]st of the mechanism is
> already present and just needs borrowing from addvars().

It looks like half closing your eyes, sticking your fingers in your
ears, and copying what's there already does the trick.

Nice to keep procedures consistent.

(By the way, if I've been following what we've been doing,

  typeset foo[3]=blah

is now guaranteed not to give you a "no match" error or bogus glob
match, though

  typeset foo[3]

isn't, but that doesn't matter as you can't change the attributes of an
array element on its own.)

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index bc68545..3da1678 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2347,9 +2347,16 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	    asg->is_array = 0;
 	    keeplocal = 0;
 	    on = pm->node.flags;
+	} else if (PM_TYPE(on) == PM_ARRAY && ASG_ARRAYP(asg)) {
+	    if (!(pm = setaparam(pname, asg->value.array ? zlinklist2array(asg->value.array) :
+				 mkarray(NULL))))
+		return NULL;
+	    asg->value.array = NULL;
+	    keeplocal = 0;
+	    on = pm->node.flags;
 	} else {
 	    zerrnam(cname,
-		    "%s: array elements must be scalar", pname);
+		    "%s: inconsistent array element or slice assignment", pname);
 	    return NULL;
 	}
     }
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 1548b81..5d69e5d 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -676,3 +676,18 @@
 >array-local
 >yes
 >no
+
+  array=(nothing to see here)
+  fn() {
+    typeset array=(one two three four five)
+    typeset array[2,4]=(umm er)
+    print ${#array} $array
+    typeset array[2,3]=()
+    print ${#array} $array
+  }
+  fn
+  print ${#array} $array
+0:can update array slices in typeset
+>4 one umm er five
+>2 one five
+>4 nothing to see here

PATCH: typeset completion

[Peter Stephenson]

On Thu, 25 Jun 2015 10:29:23 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> > This is perhaps jumping ahead a bit but for completion, it seems we now
> > get -command- context after typeset. As long as -var- context is applied
> > inside the actual contexts, having -command- context otherwise for
> > typeset would probably work fine.
> 
> That still needs looking at --- as this is a rather grungy interface
> into the main shell it's a bit of a dark art whether this should be
> tweaked by fixing up the context or fixing up the handler, or whether
> it's one of the cases where the Red Death holds illimitable sway and
> dominion.  With any luck one of the first two is possible.
> 
> Scalar and array contexts after typeset assignment seem to be working OK
> as a -value-.

I believe this fixes the major issues within the main completion code,
but just in the nick of time before the Red Death leapt out of it...

pws

diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index f18ad17..7217abc 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -1190,6 +1190,12 @@ get_comp_string(void)
 	/* Get the next token. */
 	if (linarr)
 	    incmdpos = 0;
+	/*
+	 * Arrange to parse assignments after typeset etc...
+	 * but not if we're already in an array.
+	 */
+	if (cmdtok == TYPESET)
+	    intypeset = !linarr;
 	ctxtlex();
 
 	if (tok == LEXERR) {
@@ -1272,10 +1278,11 @@ get_comp_string(void)
 	    tt0 = NULLTOK;
 	}
 	if (lincmd && (tok == STRING || tok == FOR || tok == FOREACH ||
-		       tok == SELECT || tok == REPEAT || tok == CASE)) {
+		       tok == SELECT || tok == REPEAT || tok == CASE ||
+		       tok == TYPESET)) {
 	    /* The lexer says, this token is in command position, so *
 	     * store the token string (to find the right compctl).   */
-	    ins = (tok == REPEAT ? 2 : (tok != STRING));
+	    ins = (tok == REPEAT ? 2 : (tok != STRING && tok != TYPESET));
 	    zsfree(cmdstr);
 	    cmdstr = ztrdup(tokstr);
 	    cmdtok = tok;
@@ -1290,7 +1297,7 @@ get_comp_string(void)
 	     * handle completing multiple SEPER-ated command positions on
 	     * the same command line, e.g., pipelines.
 	     */
-	    ins = (cmdtok != STRING);
+	    ins = (cmdtok != STRING && cmdtok != TYPESET);
 	}
 	if (!lexflags && tt0 == NULLTOK) {
 	    /* This is done when the lexer reached the word the cursor is on. */
@@ -1436,7 +1443,7 @@ get_comp_string(void)
 	we = wb = zlemetacs;
 	clwpos = clwnum;
 	t0 = STRING;
-    } else if (t0 == STRING) {
+    } else if (t0 == STRING || t0 == TYPESET) {
 	/* We found a simple string. */
 	s = ztrdup(clwords[clwpos]);
     } else if (t0 == ENVSTRING) {
@@ -1492,7 +1499,7 @@ get_comp_string(void)
 	zlemetaline = tmp;
 	zlemetall = strlen(zlemetaline);
     }
-    if (t0 != STRING && inwhat != IN_MATH) {
+    if (t0 != STRING && t0 != TYPESET && inwhat != IN_MATH) {
 	if (tmp) {
 	    tmp = NULL;
 	    linptr = zlemetaline;
diff --git a/Src/lex.c b/Src/lex.c
index d56f670..baeed13 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -339,6 +339,7 @@ ctxtlex(void)
 	incmdpos = 1;
 	break;
     case STRING:
+    case TYPESET:
  /* case ENVSTRING: */
     case ENVARRAY:
     case OUTPAR:

Re: Typeset with array

[Oliver Kiddle]

Peter wrote:
> Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> > Similarly: typeset -i foo=(23)
> 
> I decided to make these errors --- the user is in a good position to do
> the right thing here since it's staring them in the face and fixing
> things up in typeset by guessing what the user actually meant is (you
> can trust me on this :-/) very messy, so I think this is reasonable.

Sounds fair enough, however, in the case of something like:
  % typeset -i i=(2+3)*4
  zsh: no matches found: *4
it isn't exactly staring you in the face.

Both bash and ksh allow math expressions starting with parentheses.
They do different things in this case:
  typeset -i bob
  one=5
  typeset bob=(one)
  echo $bob

Also, note the following:
  typeset var1=(one two)var2=three
Bash treats that as a string assignment. Zsh and ksh consider that to be
two separate assignments. That's also true without the typeset. That
seems reasonable.

This is not new but what is going on here?:
 % var=x(
 >
That's a PS2 prompt where %_ is nothing.

Oliver

Re: PATCH: array slice

[Bart Schaefer]

On Jun 26,  2:51pm, Peter Stephenson wrote:
}
} It looks like half closing your eyes, sticking your fingers in your
} ears, and copying what's there already does the trick.

Very good!  Just a couple of notes ...

This error message looks odd:

torch% typeset y[2,4]=(x)
typeset: y[2,4]: can't create local array elements

I guess that isn't new, but to refer to "local" at top-level prompt is
funny.  "(on & PM_LOCAL)" is evidently true even though the typeset is
not within function scope.  Amusingly it works to simply force the
variable to be global:

torch% typeset -g y[2,4]=(x)            
torch% typeset -p y
typeset -a y
y=('' x)

And an array is always created for this, even without the parens:

torch% typeset -g z[2,4]=y
torch% typeset -p z       
typeset -a z
z=('' y)

Lastly, handling slices still doesn't fix the empty array assignment:

torch% typeset x=() x[1]=1 x[3]=3
torch% typeset -p x
typeset -a x
x=(1 '' 3)
torch% typeset x=() x[2]=b x[4]=d
torch% typeset -p x              
typeset -a x
x=(1 b 3 d)
torch% typeset x[2]=2 x[4]=4 x=()
torch% typeset -p x              
typeset -a x
x=(1 2 3 4)

Although this works:

torch% typeset x[1,-1]=()
torch% typeset -p x
typeset -a x
x=()
torch% typeset x[2]=b x[4]=d x[1,3]=()
torch% typeset -p x
typeset -a x
x=(d)

Re: Typeset with array

[Peter Stephenson]

On Fri, 26 Jun 2015 21:14:04 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Sounds fair enough, however, in the case of something like:
>   % typeset -i i=(2+3)*4
>   zsh: no matches found: *4
> it isn't exactly staring you in the face.

Yes, it's being parsed as "i=(2+3)" followed by "*4", just as

i=(2+3)*4

would always have been parsed.

That's always been a bad thing to do, though.  Unquoted parentheses and
* will always lead you astray.  You'd have got away before with
NO_NOMATCH, but relying on that isn't great practice, to say the least.

I could make array asignments without a following space an error,
probably, but as you noted that's breaking cases that used to work, and
I don't like to do it only after typeset.

pws

Re: PATCH: array slice

[Peter Stephenson]

On Fri, 26 Jun 2015 16:07:45 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> This error message looks odd:
> 
> torch% typeset y[2,4]=(x)
> typeset: y[2,4]: can't create local array elements
> 
> I guess that isn't new, but to refer to "local" at top-level prompt is
> funny.  "(on & PM_LOCAL)" is evidently true even though the typeset is
> not within function scope.  Amusingly it works to simply force the
> variable to be global:
> 
> torch% typeset -g y[2,4]=(x)            
> torch% typeset -p y
> typeset -a y
> y=('' x)
> 
> And an array is always created for this, even without the parens:
> 
> torch% typeset -g z[2,4]=y
> torch% typeset -p z       
> typeset -a z
> z=('' y)

OK, so it does work if it *would* create a local if it could but is
*actually* going to create a global in practice.  This probably isn't
all that useful in practice --- in the second case, you haven't even
told it you want an array.

It would take more work to fix this so it does something sensible if the
variable would be local --- we get into a mess later, which is what I
think the error is for.  I don't quite understand why but I've
lost interest / the will to live; there are too many combinations.

pws

--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2325,7 +2325,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	    zerrnam(cname,
 		    "%s: can't create readonly array elements", pname);
 	    return NULL;
-	} else if (on & PM_LOCAL) {
+	} else if ((on & PM_LOCAL) && locallevel) {
 	    *subscript = 0;
 	    pm = (Param) (paramtab == realparamtab ?
 			  gethashnode2(paramtab, pname) :

Re: PATCH: array slice

[Bart Schaefer]

On Jun 27,  8:42pm, Peter Stephenson wrote:
} Subject: Re: PATCH: array slice
}
} On Fri, 26 Jun 2015 16:07:45 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > 
} > torch% typeset y[2,4]=(x)
} > typeset: y[2,4]: can't create local array elements
} 
} It would take more work to fix this so it does something sensible if the
} variable would be local --- we get into a mess later, which is what I
} think the error is for.  I don't quite understand why but I've
} lost interest / the will to live; there are too many combinations.

The short rationale is that "local y[1]" APPEARS TO BE attempting to
make the single element ${y[1]} local without making the rest of the
array local.  You can't do that in any case; the entire array has to
be local for any of it to be.

The same goes for assoc arrays.  Contrast this with perl where the
real "local" confusion may come from:

% perl -le '@y=(1..4); do { local $y[2]='c'; map { print } @y }'
1
2
c
4


Administrivia:  Peter's message arrived in my inbox with

To: "Zsh Hackers' List" <zsh-workers@zsh.org.hsd1.ca.comcast.net>

My email is received at gmail, so this likely happened somewhere upstream,
possibly at the list exploder itself.  Weird.  It looks correct in the
list archives.