Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[osh] Implement ${a[@]@P} and make ${a[@]@a} and ${a[@]@Q} consistent with Bash #2210

Merged
merged 7 commits into from
Jan 2, 2025
+134 −46
Merged

[osh] Implement ${a[@]@P} and make ${a[@]@a} and ${a[@]@Q} consistent with Bash #2210

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5251613
[spec/var-op-bash] Add failing spec test for `${a[@]@...}`
akinomyoga Dec 25, 2024
ee5e3ee
[osh refactor] make prompt.EvalPrompt receive str
akinomyoga Dec 25, 2024
c493cd2
[osh] Fix `${a[@]@Q}` connecting words
akinomyoga Dec 25, 2024
91ce829
[osh] Fix `${a[@]@a}` to generate an attribute for each element
akinomyoga Dec 25, 2024
8e8271e
[osh] Implement `${a[@]@P}`
akinomyoga Dec 25, 2024
0653c5e
[spec/var-op-bash] Simplify spec test var-op-bash#16
akinomyoga Jan 2, 2025
fda3cd4
[spec/var-op-bash] Turn osh's var-op-bash#15 result to FAIL
akinomyoga Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions osh/prompt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,22 +242,18 @@ def _ReplaceBackslashCodes(self, tokens):

return ''.join(ret)

def EvalPrompt(self, UP_val):
# type: (value_t) -> str
def EvalPrompt(self, s):
# type: (str) -> str
"""Perform the two evaluations that bash does.

Used by $PS1 and ${x@P}.
"""
if UP_val.tag() != value_e.Str:
return '' # e.g. if the user does 'unset PS1'

val = cast(value.Str, UP_val)

# Parse backslash escapes (cached)
tokens = self.tokens_cache.get(val.s)
tokens = self.tokens_cache.get(s)
if tokens is None:
tokens = match.Ps1Tokens(val.s)
self.tokens_cache[val.s] = tokens
tokens = match.Ps1Tokens(s)
self.tokens_cache[s] = tokens

# Replace values.
ps1_str = self._ReplaceBackslashCodes(tokens)
Expand Down Expand Up @@ -304,7 +300,12 @@ def EvalFirstPrompt(self):
# Now try evaluating $PS1
ps1_val = self.mem.env_config.GetVal('PS1')
#log('ps1_val %s', ps1_val)
return self.EvalPrompt(ps1_val)
UP_ps1_val = ps1_val
if UP_ps1_val.tag() == value_e.Str:
ps1_val = cast(value.Str, UP_ps1_val)
return self.EvalPrompt(ps1_val.s)
else:
return '' # e.g. if the user does 'unset PS1'


PROMPT_COMMAND = 'PROMPT_COMMAND'
Expand Down
10 changes: 4 additions & 6 deletions osh/prompt_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import unittest

from _devbuild.gen.value_asdl import value
from core import state
from core import test_lib
from frontend import match
Expand All @@ -25,21 +24,20 @@ def setUp(self):

def testEvaluator(self):
# Regression for caching bug!
self.assertEqual('foo', self.p.EvalPrompt(value.Str('foo')))
self.assertEqual('foo', self.p.EvalPrompt(value.Str('foo')))
self.assertEqual('foo', self.p.EvalPrompt('foo'))
self.assertEqual('foo', self.p.EvalPrompt('foo'))

def testNoEscapes(self):
for prompt_str in ["> ", "osh>", "[[]][[]][][]]][["]:
self.assertEqual(self.p.EvalPrompt(value.Str(prompt_str)),
prompt_str)
self.assertEqual(self.p.EvalPrompt(prompt_str), prompt_str)

def testValidEscapes(self):
for prompt_str in [
"\[\033[01;34m\]user\[\033[00m\] >", r"\[\]\[\]\[\]",
r"\[\] hi \[hi\] \[\] hello"
]:
self.assertEqual(
self.p.EvalPrompt(value.Str(prompt_str)),
self.p.EvalPrompt(prompt_str),
prompt_str.replace(r"\[", "\x01").replace(r"\]", "\x02"))

def testInvalidEscapes(self):
Expand Down
44 changes: 38 additions & 6 deletions osh/word_eval.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ def _Slice(self, val, op, var_name, part):
return val

def _Nullary(self, val, op, var_name, vsub_token, vsub_state):
# type: (value_t, Token, Optional[str], Token, VarSubState) -> Tuple[value.Str, bool]
# type: (value_t, Token, Optional[str], Token, VarSubState) -> Tuple[value_t, bool]

quoted2 = False
op_id = op.id
Expand All @@ -1057,13 +1057,31 @@ def _Nullary(self, val, op, var_name, vsub_token, vsub_state):
UP_val = val
with tagswitch(val) as case:
if case(value_e.Undef):
result = value.Str('')
result = value.Str('') # type: value_t
elif case(value_e.Str):
str_val = cast(value.Str, UP_val)
prompt = self.prompt_ev.EvalPrompt(str_val)
prompt = self.prompt_ev.EvalPrompt(str_val.s)
# readline gets rid of these, so we should too.
p = prompt.replace('\x01', '').replace('\x02', '')
result = value.Str(p)
elif case(value_e.BashArray, value_e.BashAssoc):
if val.tag() == value_e.BashArray:
val = cast(value.BashArray, UP_val)
values = [
s for s in bash_impl.BashArray_GetValues(val)
if s is not None
]
elif val.tag() == value_e.BashAssoc:
val = cast(value.BashAssoc, UP_val)
values = bash_impl.BashAssoc_GetValues(val)
else:
raise AssertionError()

tmp = [
self.prompt_ev.EvalPrompt(s).replace(
'\x01', '').replace('\x02', '') for s in values
]
result = value.BashArray(tmp)
else:
e_die("Can't use @P on %s" % ui.ValType(val), op)

Expand All @@ -1078,7 +1096,10 @@ def _Nullary(self, val, op, var_name, vsub_token, vsub_state):
self._ProcessUndef(val, vsub_token, vsub_state)

# For unset variables, we do not generate any quoted words.
result = value.Str('')
if vsub_state.array_ref is not None:
result = value.BashArray([])
else:
result = value.Str('')

elif case(value_e.Str):
str_val = cast(value.Str, UP_val)
Expand All @@ -1100,7 +1121,7 @@ def _Nullary(self, val, op, var_name, vsub_token, vsub_state):
# TODO: should use fastfunc.ShellEncode
j8_lite.MaybeShellEncode(s) for s in values
]
result = value.Str(' '.join(tmp))
result = value.BashArray(tmp)
else:
e_die("Can't use @Q on %s" % ui.ValType(val), op)

Expand All @@ -1126,7 +1147,18 @@ def _Nullary(self, val, op, var_name, vsub_token, vsub_state):
if cell.nameref:
chars.append('n')

result = value.Str(''.join(chars))
count = 1
with tagswitch(val) as case:
if case(value_e.Undef):
count = 0
elif case(value_e.BashArray):
val = cast(value.BashArray, UP_val)
count = bash_impl.BashArray_Count(val)
elif case(value_e.BashAssoc):
val = cast(value.BashAssoc, UP_val)
count = bash_impl.BashAssoc_Count(val)

result = value.BashArray([''.join(chars)] * count)

else:
e_die('Var op %r not implemented' % lexer.TokenVal(op), op)
Expand Down
6 changes: 0 additions & 6 deletions spec/prompt.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,6 @@ status=0
x
status=0
## END
## OK osh STDOUT:
status=1
status=1
x
status=0
## END

#### default PS1
#flags='--norc --noprofile'
Expand Down
86 changes: 83 additions & 3 deletions spec/var-op-bash.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,11 @@ status=0
status=0
## END
## OK osh STDOUT:
status=1
a b c
status=0
a $'b\\nc'
status=0
a
a a a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that OSH differs from bash because of a quoting difference, which is OK

But it also seems to differ with a a a vs. '' ?

Or maybe that is addressed in a follow-up?

TBH I don't understand the reasons for the evaluation logic - in these cases, my review falls back to "does OSH match bash?"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Thanks for pointing out this. This is just my oversight. OSH should generate the same result as Bash. Let me think about how we should distinguish array-origin BashArray from $@-origin BashArray.

Copy link
Contributor

@andychu andychu Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is OK if we fix that later (or even not at all if nobody runs into it, and if bash does not document it)

Sometimes when I encounter a quirk like this, I split it into a separate test case, and then mark it oils_failures_allowed += 1

That is just a "note for later" (Basically because there seem to be infinite bash implementation details! I think using @ with a type query is EXTREMELY obscure. I will be surprised if anyone depends on this)

So that way the first test case have bash STDOUT === osh STDOUT, which is a nice proprety

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you wait for a while? If it turns out to be easy to fix, I can fix it within this PR. If not, I'll handle it in a separate PR later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to defer it. The best solution could be to introduce a new type to represent the list coming from $@, but one might relate it to the forthcoming switching from BashArray to SparseArray. We can discuss that later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now this is an allowed OSH failure: fda3cd4

status=0
## END

Expand All @@ -328,7 +329,8 @@ A - A
status=0
## END
## OK osh STDOUT:
status=1
- y
akinomyoga marked this conversation as resolved.
Show resolved Hide resolved
status=0
- y
status=0
A - A
Expand Down Expand Up @@ -472,3 +474,81 @@ a
A
A
## END


#### Array expansion with nullary var ops

declare -a a=({1..9})
declare -A A=(['a']=hello ['b']=world ['c']=osh ['d']=ysh)

echo "@Q"
argv.py "${a[@]@Q}"
argv.py "${a[*]@Q}"
argv.py "${A[@]@Q}"
argv.py "${A[*]@Q}"
argv.py "${u[@]@Q}"
argv.py "${u[*]@Q}"

echo "@P"
argv.py "${a[@]@P}"
argv.py "${a[*]@P}"
argv.py "${A[@]@P}"
argv.py "${A[*]@P}"
argv.py "${u[@]@P}"
argv.py "${u[*]@P}"

echo "@a"
argv.py "${a[@]@a}"
argv.py "${a[*]@a}"
argv.py "${A[@]@a}"
argv.py "${A[*]@a}"
argv.py "${u[@]@a}"
argv.py "${u[*]@a}"

## STDOUT:
@Q
['1', '2', '3', '4', '5', '6', '7', '8', '9']
['1 2 3 4 5 6 7 8 9']
['hello', 'world', 'osh', 'ysh']
['hello world osh ysh']
[]
['']
@P
['1', '2', '3', '4', '5', '6', '7', '8', '9']
['1 2 3 4 5 6 7 8 9']
['hello', 'world', 'osh', 'ysh']
['hello world osh ysh']
[]
['']
@a
['a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a']
['a a a a a a a a a']
['A', 'A', 'A', 'A']
['A A A A']
[]
['']
## END

## OK bash STDOUT:
@Q
["'1'", "'2'", "'3'", "'4'", "'5'", "'6'", "'7'", "'8'", "'9'"]
akinomyoga marked this conversation as resolved.
Show resolved Hide resolved
["'1' '2' '3' '4' '5' '6' '7' '8' '9'"]
["'ysh'", "'osh'", "'world'", "'hello'"]
["'ysh' 'osh' 'world' 'hello'"]
[]
['']
@P
['1', '2', '3', '4', '5', '6', '7', '8', '9']
['1 2 3 4 5 6 7 8 9']
['ysh', 'osh', 'world', 'hello']
['ysh osh world hello']
[]
['']
@a
['a', 'a', 'a', 'a', 'a', 'a', 'a', 'a', 'a']
['a a a a a a a a a']
['A', 'A', 'A', 'A']
['A A A A']
[]
['']
## END
3 changes: 1 addition & 2 deletions spec/var-ref.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
## compare_shells: bash
## oils_failures_allowed: 1

# Var refs are done with ${!a}
#
Expand Down Expand Up @@ -744,7 +743,7 @@ test-op0 'a3[@]'
# []
# []

## BUG bash STDOUT:
## OK bash STDOUT:
==== v1 ====
["'value'"]
['value']
Expand Down