Skip to content

install.sh: Fix -h handling and error on unrecognized parameters#252

Merged
dlang-bot merged 2 commits intodlang:masterfrom
CyberShadow:pull-20170907-204740
Sep 7, 2017
Merged

install.sh: Fix -h handling and error on unrecognized parameters#252
dlang-bot merged 2 commits intodlang:masterfrom
CyberShadow:pull-20170907-204740

Conversation

@CyberShadow
Copy link
Member

No description provided.

Fix "./install.sh: line 161: $1: unbound variable" when running
install.sh with -h or --help.

Regression introduced in commit
5793069.
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @CyberShadow!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.


*)
usage
fatal "Unrecognized command-line parameter: $1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe first showing the error message first?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I intentionally put it after the usage text. You're more likely to notice stuff at the bottom of your terminal window (near the prompt) than near the top.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, I wouldn't have minded anyways

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it's weird that most tools do it vice versa.

@dlang-bot dlang-bot merged commit 1399958 into dlang:master Sep 7, 2017
@jondegenhardt
Copy link

jondegenhardt commented Sep 26, 2017

This PR appears to have exposed a bug in the install script. The argument parsing for -p | --path (https://github.com/dlang/installer/blob/master/script/install.sh#L254-L259) takes a second argument, the path. However, the '-p|--path' argument handler doesn't actually consume the argument, which then get run through arg processing loop. Previously it was ignored. Now the script exits with the "Unrecognized command-line parameter" message.

I found this trying to run the docker-ldc script, which uses the -p option: https://github.com/lindt/docker-ldc/blob/master/Dockerfile#L11

Update: Bugzilla issue: https://issues.dlang.org/show_bug.cgi?id=17858

@CyberShadow
Copy link
Member Author

Looks like Martin had fixed this by the time you ran into it: 143bbf8

@MartinNowak
Copy link
Member

Was already wondering how that could have been there forever ;).

@jondegenhardt
Copy link

And thanks for putting the error message at the bottom of the window!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants