mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.1k stars 336 forks source link

interp: display all Bash's `shopt` option #883

Closed riacataquian closed 2 years ago

riacataquian commented 2 years ago

Trying to set an unsupported but valid Bash option leads to a potentially confusing error message:

$ gosh -c "shopt -s extglob"
shopt: invalid option name "extglob"

Fix that by handling the unsupported options differently from the invalid ones:

$ gosh -c "shopt -s extglob"
bash: line 1: shopt: extglob off ("on" not supported)
exit status 1

Additionally, this commit lists all of the Bash options when shopt without arguments is called and explicitly identify the unsupported options, for example:

$ gosh -c "shopt"
expand_aliases  off
globstar        off
nullglob        off
// .. cut for brevity
hostcomplete    on      ("off" not supported)
inherit_errexit on      ("off" not supported)
interactive_comments    on      ("off" not supported)

While at it, rewrite the bashOptsTable so that it can keep two option states: 1) Bash's default options and 2) whether we support it

Fixes #877

mvdan commented 2 years ago

Rather than keeping two booleans for "supported" and "allow override", how about:

default bool // default of "off" or "on" like Bash
supported bool // whether we support the option's non-default state

I think that's a bit easier to understand, and it should be enough information to print what we need:

1) If an option is supported, we simply print the name and its default, like globstar off. 2) If an option is not supported and its default is off, we print globstar off ("on" not supported). 3) If an option is not supported and its default is on, we print globstar on ("off" not supported).

This output is also clearer to the end user, I think.

mvdan commented 2 years ago

I also think we need to record the default values anyway, as I'm fairly sure there are a good number of Bash options that default to "on".

riacataquian commented 2 years ago

That sounds better, thanks! :)

Also, I agree on:

I also think we need to record the default values anyway, as I'm fairly sure there are a good number of Bash options that default to "on".

I briefly mentioned it here https://github.com/mvdan/sh/pull/883#discussion_r901299481, will also include in this PR.

riacataquian commented 2 years ago

@mvdan could you take a look please? I incorporated the feedbacks above. Thanks!