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

Fix install script for #213 #214

Merged
merged 5 commits into from
Dec 29, 2021

Conversation

akinomyoga
Copy link
Contributor

Fix #213. There are several issues in the current install script. This PR tries to fix them. I separated commits for each fixes as follows. Thank you.

Add support for termcap-based tput 02620c8

As I have already commented in ***, tput may support termcap names instead of terminfo names depending on the system.

Also, I have changed which to hash as is later used to test git. which is not a POSIX utility, so it may be missing. hash is a builtin command of Bash so always available while running in Bash.

The right-hand sides of assignments is not subject to word splitting or pathname expansions, so quotation is unneeded for the command substitutions.

Fix the version check 03ee310

[ -n $BASH_VERSION ]

In Bash and POSIX shells (unlike the default of Zsh), the unquoted parameter expansion $BASH_VERSION is subject to word splitting and pathname expansions. When BASH_VERSION is an empty string, it will be [ -n ]. When [ ... ] receives only a single argument, it just tests whether the argument is non-empty. It just tests if the string -n is non-empty, which is always true. One needs to quote $BASH_VERSION.

Also, we should first test the shell version outside the function because less-functional shells may fail to parse the function itself before it checks the shell version in the function. For example, the original Bourne shell doesn't understand the command substitution of the form $(...).

In case it matters, the version check code was introduced by e66772b.

Quote arguments properly 599e277

In Bash and POSIX shells (unlike the default of Zsh), the unquoted arguments undergo word splitting and pathname expansions. For example, the unquoted $HOME will be split to multiple words when HOME contains spaces or when IFS contains characters in HOME.

Also, instead of quoting $HOME, one may use ~ in the beginning of words and right-hand sides of assignments.

Use conditional command 2f2a12d

In Bash, instead of [, it is recommended to use the special compound command [[ ... ]], for which the syntactic treatment of the arguments are different from the one in the normal context.

@akinomyoga
Copy link
Contributor Author

I'm thinking of merging this soon because there seem like several issues and PRs related to install.sh including #213, #236, and #267. I would probably be going to work on these issues after settling existing PRs because I don't want to make unnecessary conflicts with these existing efforts.

@akinomyoga akinomyoga closed this Dec 23, 2021
@akinomyoga akinomyoga deleted the fix-install-script branch December 23, 2021 21:22
@akinomyoga akinomyoga restored the fix-install-script branch December 23, 2021 21:23
@akinomyoga akinomyoga reopened this Dec 23, 2021
@akinomyoga
Copy link
Contributor Author

#239 is also touching install.sh

@akinomyoga akinomyoga merged commit f647fb4 into ohmybash:master Dec 29, 2021
@akinomyoga akinomyoga deleted the fix-install-script branch December 29, 2021 00:08
@akinomyoga
Copy link
Contributor Author

I have merged the PR because there are other awaiting PRs related to install.sh

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

Successfully merging this pull request may close these issues.

sh: 33: [: Illegal number:
1 participant