Fix broken getopts() usage, allows multiple flags #286
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
The current usage of getopts is a kind of dirty hack: using it to
iterate over passable arguments but using a hand rolled flag matching
syntax. It appears this was done as a workaround to catch a long form
‘--debug’ argument, but I believe it introduces more problems than it
solves. The long form argument does sort of work, but only after getpots
throws an error about invalid argument syntax and only on a fluke of
‘-d’ being a substring of ‘--debug’ and getpots having assigned ‘ebug’
as $OPTARG.
In one sense this is a regression because it removes the long form
option parsing, but it was not documented in help anyway and I believe
it is unlikely anybody is using this particular flag in a script that
will break.
On the other hand with proper iteration of the getopts assigned flags
variable, we can now properly handle multiple flags. Previously things
like `-d -v` or `-v -c foo` would attempt to do crazy broken things such
as:
GIT_DIR=/home/caleb/.config/vcsh/repo.d/-v.git
Now multiple flag handling just works as expected.
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.
The current usage of getopts is a kind of dirty hack: using it to iterate over passable arguments but using a hand rolled flag matching syntax to actually parse them. It appears this was done as a workaround to catch a long form ‘--debug’ argument, but I believe it introduces more problems than it solves. The long form argument does sort of work, but only after getpots throws an error about invalid argument syntax and only on a fluke of ‘-d’ being a substring of ‘--debug’ and getpots having assigned ‘ebug’ as $OPTARG.
In one sense this is a regression because it removes the long form option parsing, but it was not documented in help anyway and I believe it is unlikely anybody is using this particular flag in a script that will break.
On the other hand with proper iteration of the getopts assigned flags variable, we can now properly handle multiple flags. Previously things like
-d -vor-v -c foowould attempt to do crazy broken things such as:Now multiple flag handling just works as expected.