Skip to content

Commit

Permalink
[ysh breaking] ARGV is a normal List variable
Browse files Browse the repository at this point in the history
It's not an alias for the "argv stack".

Why?

- There was a bug where mutating ARGV was ignored.  You got a new
  value.List every time.
- Removing the argv stack simplifies a pure YSH runtime (see Zulip)
- Removing the argv stack removes special semantics from the language.
  - ARGV is just a list.
- We are going to get rid of "$@", so there's no reason to be
  constrained by its odd semantics.

Now these are 2 different "worlds":

- shell functions and "$@
- procs and @argv
  • Loading branch information
Andy Chu committed Apr 13, 2024
1 parent f1fe1c6 commit 1946733
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 41 deletions.
26 changes: 10 additions & 16 deletions core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -961,15 +961,14 @@ def __init__(self, mem, mutable_opts, proc, argv):
frame = NewDict() # type: Dict[str, Cell]

assert argv is not None
if True:
if proc.sh_compat:
# shell function
mem.argv_stack.append(_ArgFrame(argv))
else:
# TODO: procs get argv
# - open: the args
# - closed: always empty
#frame['ARGV'] = _MakeArgvCell(argv)
pass
# procs
# - open: is equivalent to ...ARGV
# - closed: ARGV is empty list
frame['ARGV'] = _MakeArgvCell(argv)

mem.var_stack.append(frame)

Expand All @@ -982,6 +981,7 @@ def __init__(self, mem, mutable_opts, proc, argv):
# 'if p' should be allowed.
self.mem = mem
self.mutable_opts = mutable_opts
self.sh_compat = proc.sh_compat

def __enter__(self):
# type: () -> None
Expand All @@ -993,8 +993,8 @@ def __exit__(self, type, value, traceback):
self.mem.PopCall()
self.mem.var_stack.pop()

# TODO: do this conditionally
self.mem.argv_stack.pop()
if self.sh_compat:
self.mem.argv_stack.pop()


class ctx_Temp(object):
Expand Down Expand Up @@ -1112,7 +1112,7 @@ def __init__(self, dollar0, argv, arena, debug_stack):

frame = NewDict() # type: Dict[str, Cell]

#frame['ARGV'] = _MakeArgvCell(argv)
frame['ARGV'] = _MakeArgvCell(argv)

self.var_stack = [frame]

Expand Down Expand Up @@ -1879,14 +1879,8 @@ def GetValue(self, name, which_scopes=scope_e.Shopt):
# if name not in COMPUTED_VARS: ...

with str_switch(name) as case:
if case('ARGV'):
# TODO: ARGV can be a normal mutable variable in YSH
items = [value.Str(s)
for s in self.GetArgv()] # type: List[value_t]
return value.List(items)

# "Registers"
elif case('_status'):
if case('_status'):
return num.ToBig(self.TryStatus())

elif case('_error'):
Expand Down
4 changes: 2 additions & 2 deletions spec/ysh-builtin-module.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ status=0
## END

#### runproc
shopt --set parse_proc
shopt --set parse_proc parse_at

f() {
write -- f "$@"
}
proc p {
write -- p "$@"
write -- p @ARGV
}
runproc f 1 2
echo status=$?
Expand Down
43 changes: 27 additions & 16 deletions spec/ysh-proc.test.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## oils_failures_allowed: 1
## oils_failures_allowed: 0

#### Open proc (any number of args)
shopt --set parse_proc
Expand Down Expand Up @@ -42,7 +42,7 @@ builtin set -- a b c
foo x y z
## STDOUT:
ARGV x y z
dollar-at x y z
dollar-at a b c
## END

#### Closed proc has empty "$@" or ARGV
Expand All @@ -60,7 +60,7 @@ params
x
y
z
['dollar-at']
['dollar-at', 'a', 'b', 'c']
['ARGV']
## END

Expand Down Expand Up @@ -157,10 +157,10 @@ p x (:| a b |, {bob: 42}, a = 5)
## END

#### Proc name-with-hyphen
shopt --set parse_proc
shopt --set parse_proc parse_at

proc name-with-hyphen {
echo "$@"
echo @ARGV
}
name-with-hyphen x y z
## STDOUT:
Expand Down Expand Up @@ -369,24 +369,24 @@ argv.py @ARGV

set -- 'a b' c
argv.py "$@"
argv.py @ARGV
argv.py @ARGV # separate from the argv stack

f() {
argv.py "$@"
argv.py @ARGV
argv.py @ARGV # separate from the argv stack
}
f 1 '2 3'
## STDOUT:
[]
[]
['a b', 'c']
['a b', 'c']
['1', '2 3']
[]
['1', '2 3']
[]
## END


#### Mutating global and local ARGV
#### Mutating global ARGV

$SH -c '
shopt -s ysh:upgrade
Expand All @@ -396,12 +396,24 @@ argv.py global @ARGV
# should not be ignored
call ARGV->append("GG")
argv.py global @ARGV
'
## STDOUT:
['global']
['global', 'GG']
## END

#### Mutating local ARGV

$SH -c '
shopt -s ysh:upgrade
argv.py global @ARGV
proc p {
argv.py local @ARGV
argv.py @ARGV
call ARGV->append("LL")
argv.py local @ARGV
argv.py @ARGV
}
p local @ARGV
Expand All @@ -412,9 +424,8 @@ argv.py global @ARGV

## STDOUT:
['global', 'a b', 'c']
['global', 'a b', 'c', 'GG']
['local', 'a b', 'c', 'GG']
['local', 'a b', 'c', 'GG', 'LL']
['global', 'a b', 'c', 'GG', 'LL']
['local', 'a b', 'c']
['local', 'a b', 'c', 'LL']
['global', 'a b', 'c']
## END

8 changes: 4 additions & 4 deletions spec/ysh-scope.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -429,12 +429,12 @@ p2 x=
## END

#### unset composes when you turn on dynamic scope
shopt -s oil:all
shopt -s ysh:all

proc unset-two {
proc unset-two (v, w) {
shopt --set dynamic_scope {
unset $1
unset $2
unset $v
unset $w
}
}

Expand Down
6 changes: 3 additions & 3 deletions spec/ysh-xtrace.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ sed --regexp-extended 's/[[:digit:]]{2,}/12345/g' err.txt >&2
## END

#### proc and shell function
shopt --set oil:upgrade
shopt --set ysh:upgrade
set -x

shfunc() {
: $1
}

proc p {
: $1
proc p (x) {
: $x
}

shfunc 1
Expand Down

0 comments on commit 1946733

Please sign in to comment.