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

Treat quoted @ as part of the variable name rather than explosion marker #1535

Open
bugsbugsbux opened this issue May 2, 2022 · 4 comments

Comments

@bugsbugsbux
Copy link

note the comments

~> echo $version
0.18.0
~> var foo = [a b c]
~> var '@bar' = [x y z]

~> echo $foo $'foo' $@foo  # ok
[a b c] [a b c] a b c
~> echo $@'foo'  # imo: should work
compilation error: 5-7 in [tty]: variable $ not found
compilation error: variable $ not found
[tty 5], line 1: echo $@'foo'
~> echo $'@foo' # should error with 'variable $"@foo" not found'
a b c
~> echo $@bar   # should error with 'variable $bar not found'
[x y z]
~> echo $'@bar'
[x y z]
~> echo $'@@bar'
compilation error: 5-13 in [tty]: variable $@bar not found
compilation error: variable $@bar not found
[tty 10], line 1: echo $'@@bar'
~> echo $@'@bar'
compilation error: 5-7 in [tty]: variable $ not found
compilation error: variable $ not found
[tty 11], line 1: echo $@'@bar'

regards

@krader1961
Copy link
Contributor

~> echo $@'foo'  # imo: should work

Why should that "work"? It is a compounding expression. I'll grant you that the documentation of variables could benefit from some more verbiage regarding quoted variable names. In particular, the current behavior is alluded to here. The former could possibly benefit from a reference to the latter. And the latter would benefit from an explicit mention of compound expressions not being valid variable names. Nonetheless, I think the current behavior is sensible and shouldn't be changed -- at least not without significant discussion as to the benefits versus possible ambiguities. Therefore, it seems to me this is really an issue about improving the documentation rather than changing the behavior.

@hanche
Copy link
Contributor

hanche commented Jun 22, 2022

It is a compounding expression

Yes, but only because it is parsed that way, and it is not obvious that it should. I think the current parsing rule is misguided. The way I think variable references ought to be parsed, is as follows:

A literal dollar sign $, optionally followed by an at sign @, followed by a variable name (quoted or unquoted as appropriate).

Note that my proposal does not permit the “exploding indicator” @ to be quoted. Thus $'@foo' looks for a variable named @foo and does not explode it, whereas $@'foo' looks for a variable named 'foo' and explodes it. And $@'@foo' looks for a variable named @foo and explodes it.

I think this bevaviour is much more coherent than the current one.

@krader1961
Copy link
Contributor

I've changed my mind and agree with @hanche. In fact, looking at the original problem statement this is clearly wrong:

> var '@bar' = yes
> echo $'@@bar'
compilation error: 5-13 in [interactive]: variable $@bar not found
compilation error: variable $@bar not found
[tty 2], line 1: echo $'@@bar'

That error message should say variable $'@@bar' not found. Note the quoting and two, not one, at symbol.

@xiaq
Copy link
Member

xiaq commented Jan 16, 2024

The current implementation of $@varname is a bit of a hack - the @ is parsed as part of the variable name and seperated out later during compilation. Fully "syntaxizing" it sounds like a good idea, although one also needs to fix the assignment commands (var, set, tmp) to follow the same syntax so it's not exactly trivial.

The support for quoted words after $ was added mainly because there were builtin arithmetics commands like * and there needed to be a way to refer to their corresponding variables (in this case $'*~'). Outside of this use case I consider the use of quoted words after $ somewhat niche, so fixing this won't be high-priority for me for now.

@xiaq xiaq changed the title exploding breaks on @varname vars Treat quoted @ as part of the variable name rather than explosion marker Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants