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

interp: panics on extglob nodes #841

Closed
theclapp opened this issue Apr 4, 2022 · 2 comments · Fixed by #875
Closed

interp: panics on extglob nodes #841

theclapp opened this issue Apr 4, 2022 · 2 comments · Fixed by #875
Assignees
Labels

Comments

@theclapp
Copy link
Sponsor Collaborator

theclapp commented Apr 4, 2022

*(/) is zsh for "* but only for directories". Which it is of course fine if gosh doesn't handle, but it shouldn't panic, I wouldn't think.

% gosh
$ echo *(/)

panic: unhandled word part: *syntax.ExtGlob

goroutine 1 [running]:
mvdan.cc/sh/v3/expand.(*Config).wordFields(0xc000092000, {0xc000134450, 0x1, 0xc000028200})
	/Users/lmc/src/goget/src/github.com/mvdan/sh/expand/expand.go:637 +0x9e6
mvdan.cc/sh/v3/expand.Fields(0x1013746, {0xc000096010, 0x2, 0x0})
	/Users/lmc/src/goget/src/github.com/mvdan/sh/expand/expand.go:405 +0x212
mvdan.cc/sh/v3/interp.(*Runner).fields(0xc00012c280, {0xc000096010, 0xc00001c170, 0xc000132ba0})
	/Users/lmc/src/goget/src/github.com/mvdan/sh/interp/runner.go:170 +0x2d
mvdan.cc/sh/v3/interp.(*Runner).cmd(0xc00012c280, {0x11811b8, 0xc00001c170}, {0x1180af0, 0xc000132b70})
	/Users/lmc/src/goget/src/github.com/mvdan/sh/interp/runner.go:342 +0x1e35
mvdan.cc/sh/v3/interp.(*Runner).stmtSync(0xc00012c280, {0x11811b8, 0xc00001c170}, 0xc000136108)
	/Users/lmc/src/goget/src/github.com/mvdan/sh/interp/runner.go:288 +0x2b5
mvdan.cc/sh/v3/interp.(*Runner).stmt(0xc00012c280, {0x11811b8, 0xc00001c170}, 0xc000136108)
	/Users/lmc/src/goget/src/github.com/mvdan/sh/interp/runner.go:269 +0x176
mvdan.cc/sh/v3/interp.(*Runner).Run(0xc00012c280, {0x11811b8, 0xc00001c170}, {0x1180758, 0xc000136108})
	/Users/lmc/src/goget/src/github.com/mvdan/sh/interp/api.go:543 +0x11f
@mvdan
Copy link
Owner

mvdan commented Apr 6, 2022

FYI this is bash's extglob, which you can find in its man page; we don't support zsh yet.

The interpreter doesn't support expanding that syntax, simply because this feature is very rarely used, so it's not been a priority. I guess we could make the interpreter error instead of panic, though - would that help? I would happily approve and merge a PR doing that, as I agree that having the interpreter panic over a known TODO is a bit dramatic.

@mvdan mvdan changed the title Gosh panics on echo *(/) interp: panics on extglob nodes Apr 6, 2022
@mvdan mvdan added the bash label Apr 6, 2022
@mvdan
Copy link
Owner

mvdan commented Apr 7, 2022

For the sake of clarity, I'd be happy to review a PR implementing this change with tests, if anyone feels like taking a stab.

@riacataquian riacataquian self-assigned this Jun 5, 2022
riacataquian added a commit to riacataquian/sh that referenced this issue Jun 10, 2022
we currently don't support bash's `extglob` option and when attempted,
interp panics

fix expand so that we return the error properly and exit with status `1`

fixes mvdan#841
mvdan pushed a commit that referenced this issue Jun 10, 2022
we currently don't support bash's `extglob` option and when attempted,
interp panics

fix expand so that we return the error properly and exit with status `1`

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

Successfully merging a pull request may close this issue.

3 participants