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

syntax: $var#... is incorrectly parsed as a comment #1003

Closed
emilazy opened this issue May 23, 2023 · 8 comments
Closed

syntax: $var#... is incorrectly parsed as a comment #1003

emilazy opened this issue May 23, 2023 · 8 comments

Comments

@emilazy
Copy link

emilazy commented May 23, 2023

Example using shfmt:

emily@yuyuko ~> cat test.bash
foo=abc
bar=def
echo foo#bar
echo foo#$bar
echo $foo#bar
echo $foo#$bar
emily@yuyuko ~> bash test.bash
foo#bar
foo#def
abc#bar
abc#def
emily@yuyuko ~> shfmt test.bash
foo=abc
bar=def
echo foo#bar
echo foo#$bar
echo $foo #bar
echo $foo #$bar
emily@yuyuko ~> shfmt test.bash | bash
foo#bar
foo#def
abc
abc

None of these contain comments, but the latter two examples get turned into comments (and this actually came up in practice; see bouk/babelfish#23). I'm not sure if it's just variable substitutions or if there are other circumstances where this incorrect parse happens.

@bouk
Copy link
Contributor

bouk commented May 23, 2023

Seems like adding a spaced check here is all that's needed:

case '#':

@mvdan
Copy link
Owner

mvdan commented May 30, 2023

Nice find - genuinely surprised that noone spotted this bug before. @bouk has the right idea, but unfortunately the fix isn't that easy. I'm investigating.

@mvdan mvdan changed the title $var#... in argument lists is parsed incorrectly syntax: $var#... is incorrectly parsed as a comment May 30, 2023
@mvdan mvdan closed this as completed in 829b814 Jun 10, 2023
@mvdan
Copy link
Owner

mvdan commented Jun 10, 2023

I spent quite a bit of time overthinking different ways to fix this, but I landed on something relatively simple: a small variation on @bouk's idea. It's not a complete fix, as two other cases still don't parse the way they should, but the cases you showed in this issue are all fixed.

@emilazy
Copy link
Author

emilazy commented Jun 13, 2023

Thanks for this! Would it be good to open an issue for the remaining cases? I haven't seen nix shell $(calculate-flake-ref)#... type constructions appear in a script yet, but I think more out of luck than anything else.

@mvdan
Copy link
Owner

mvdan commented Jun 14, 2023

I left TODOs in the code as a personal reminder, but please feel free to open another issue if you think you'll run into it soon. Particularly if you already have any real example :)

@emilazy
Copy link
Author

emilazy commented Jun 14, 2023

I'll wait to run into it :)

@bouk
Copy link
Contributor

bouk commented Jun 15, 2023

@mvdan if you cut a release I'll update babelfish!

@mvdan
Copy link
Owner

mvdan commented Jun 18, 2023

done :)

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

3 participants