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.

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

cell sublanguage: unset -v 'a[0]' (ble.sh) #651

Closed
akinomyoga opened this issue Mar 9, 2020 · 8 comments
Closed

cell sublanguage: unset -v 'a[0]' (ble.sh) #651

akinomyoga opened this issue Mar 9, 2020 · 8 comments

Comments

@akinomyoga
Copy link
Collaborator

akinomyoga commented Mar 9, 2020

I use this construct in ble.sh. I could find some test case for this here and also some mentions on unset a[0] here, but it doesn't work with the current Oil. Is this a regression?

$ bin/osh -c 'a=(x y z); unset "a[1]"'
  a=(x y z); unset "a[1]"
                   ^
[ -c flag ]:1: 'unset' got invalid variable name 'a[1]'
[ble: exit 2]
$ bin/osh -c "a=(x y z); unset 'a[1]'"
  a=(x y z); unset 'a[1]'
                   ^
[ -c flag ]:1: 'unset' got invalid variable name 'a[1]'
$
@andychu
Copy link
Contributor

andychu commented Mar 9, 2020

OK we may need to do dynamic parsing of a[const] within several constructs:

  • unset
  • ${!ref}
  • declare -n (nameref)

And maybe a few other places... @abathur also used this with -n.

It will parse a[const] but not a[1+2] since the latter would parse arbitrary code.

Somewhere on the blog I called this the "cell sublanguage", oh yeah here: http://www.oilshell.org/blog/2019/02/07.html#appendix-a-minor-sublanguages

@andychu
Copy link
Contributor

andychu commented Mar 9, 2020

#608 is the feature for nameref

@andychu andychu changed the title unset -v 'a[0]' not working (ble.sh) cell language: unset -v 'a[0]' (ble.sh) Mar 9, 2020
@andychu andychu changed the title cell language: unset -v 'a[0]' (ble.sh) cell sublanguage: unset -v 'a[0]' (ble.sh) Mar 9, 2020
@andychu
Copy link
Contributor

andychu commented Mar 15, 2020

note: marked with two stars on #653

@andychu
Copy link
Contributor

andychu commented Mar 15, 2020

May relate to IR in #604 , e.g. sh_lhs_expr

andychu pushed a commit that referenced this issue Mar 18, 2020
It shouldn't be N-I since we may implement it.

Addresses issue #651.  Also tests a corner case from #661.
@andychu
Copy link
Contributor

andychu commented Mar 31, 2020

@akinomyoga Another thing I wanted to ask is if you're using only indexed arrays for

unset -v a[0]

Or if you're also using associative arrays? The latter is trickier

$ declare -A A=(['foo']=bar)
$ echo ${A[@]}
bar
$ unset -v "A['foo']"
-bash: unset: A['foo']: bad array subscript

$ unset -v "A[foo]"
$ echo ${A[@]}
(empty)

It doesn't like the quotes, and prefers bare word syntax, which is inconsistent with the rest of Oil (and bash to some extent)

@akinomyoga
Copy link
Collaborator Author

Thank you.

Another thing I wanted to ask is if you're using only indexed arrays

Yes, I searched in ble.sh source and it seems unset is currently only used for indexed arrays. But that's not intentional and I'm not sure if I will never use it forever. And, it is a little bit awkward that there is no means to remove a key from associative arrays.

It doesn't like the quotes,

Which version of Bash do you use? I tried with versions of Bash and found that the behavior was changed since 4.4. I think it was a bug of Bash.

$ bash --norc # bash-4.4 or 5.0
$ declare -A A=(['foo']=bar)
$ declare -p A
declare -A A=([foo]="bar" )
$ unset -v "A['foo']"
$ declare -p A
declare -A A=()

The safe way for unsetting an associative array element in Bash that should work with an arbitrary key is the following. I think Oil can support this form.

$ declare -A d
$ key="a]b'c[d"   # A key containing special characters
$ unset -v 'd[$key]'

andychu pushed a commit that referenced this issue Mar 31, 2020
- unset -v
- declare -n
- ${!ref}

Not done: print -v

Addresses issue #651.

Unrelated: add some ideas in stdlib/README.md
@andychu
Copy link
Contributor

andychu commented Mar 31, 2020

OK yes, I was testing on my machine which is bash 4.3. I did it through the spec tests and bash behaves consistently with associatve arrays for unset -v, declare -n, and ${!ref} ( see the last 3 test cases in commit above).

So I think we can probably do it, though I might start with just a[0] and see what we can get to run first.

andychu pushed a commit that referenced this issue Apr 4, 2020
It's hidden behind shopt -s unsafe_arith_eval to avoid surprises like:

    untrusted='A[$(echo K)]'
    unset -v "$untrusted"

The code still needs refactoring.

Also add shopt -s strict_unset, though it's not used yet.

Addresses issue #651.
andychu pushed a commit that referenced this issue Apr 4, 2020
Removed 'strict_unset' since we don't need it yet.

Related to issue #651.
@andychu
Copy link
Contributor

andychu commented Apr 4, 2020

OK this can be tested from master... Looking at #640 now (which is different than #648 in Oil)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants