interp: display all Bash's shopt option
#883
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
riacataquian
commented
Jun 20, 2022
|
Rather than keeping two booleans for "supported" and "allow override", how about: I think that's a bit easier to understand, and it should be enough information to print what we need:
This output is also clearer to the end user, I think. |
|
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". |
|
That sounds better, thanks! :) Also, I agree on:
I briefly mentioned it here #883 (comment), will also include in this PR. |
riacataquian
force-pushed
the
fix-877
branch
2 times, most recently
from
3db2bba
to
85f4362
Compare
|
@mvdan could you take a look please? I incorporated the feedbacks above. Thanks! |
mvdan
approved these changes
Jun 28, 2022
riacataquian
force-pushed
the
fix-877
branch
3 times, most recently
from
f8b9ca3
to
7a93c0c
Compare
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 mvdan#877
scop
pushed a commit
to scop/sh
that referenced
this pull request
Sep 24, 2022
A recent commit, 1eb295c, introduced a regression on Windows. Programs spawned by interp no longer found other programs in PATH. The reason turns out to be because those programs saw a Unix-like PATH, with the list separator being ":" rather than ";", which breaks Windows programs as they see one single mangled directory. The actual source of the bug is much older, from eb75bb8f7d: For better compatibility with non-unix systems, convert $PATH to a unix path list when starting the interpreter. This should work with Windows, for example. The recent commit surfaced the bug by tweaking the environment logic. Before, converting PATH's list separators was an internal detail, but now we also export this change and it affects spawned programs. The added test case fails without the fix: --- FAIL: TestRunnerRun/mvdan#883 (0.19s) interp_test.go:3201: wrong output in "GOSH_CMD=lookpath $GOSH_PROG": want: "cmd found\n" got: "exec: \"cmd\": executable file not found in %PATH%\nexit status 1" The fix is relatively simple: stop converting PATH's list separators. I believe the reason I originally added that code was for compatibility, so that scripts using Unix-like path list separator, such as export PATH="${PATH}:${PWD}" would work as expected on Windows. However, I struggle to agree with my past self on that approach. We had no tests for this behavior, and any script using Unix-like paths in any non-trivial way would likely need to be adjusted to be compatible with Windows anyway. Fixes mvdan#768.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trying to set an unsupported but valid Bash option leads to a
potentially confusing error message:
Fix that by handling the unsupported options differently from the
invalid ones:
Additionally, this commit lists all of the Bash options when
shoptwithout arguments is called and explicitly identify the unsupported
options, for example:
While at it, rewrite the
bashOptsTableso that it can keep two optionstates: 1) Bash's default options and 2) whether we support it
Fixes #877