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

Least changes needed to let Shellcheck parse to end #202

Open
wants to merge 2 commits into
base: Beta
Choose a base branch
from

Conversation

gfoury
Copy link
Contributor

@gfoury gfoury commented Feb 24, 2016

Shellcheck is a tool for static analysis of various kinds of shell scripts. It has really helped me in the past. I would like it to run from beginning to end on ssdtPRGen.sh.

tl;dr:

Shellcheck gets stuck on some lines it considers syntax errors: parenthesized let expressions without quotes. This patch adds the quotes. You don't need to take the patch; you could use this branch in my repository to run Shellcheck.

Background

In ksh (I know we're in bash!) the following is a syntax error:

#!/bin/ksh
let a=(b*c)

whereas this is ok:

#!/bin/ksh
let a="(b*c)"

Shellcheck will barf and die on the first, even when it is checking Bash scripts. Shellcheck may be right or wrong, but in order to get all its other functionality, a few let expressions in ssdtPRGen.sh need to be tweaked. I decided the least-possible-change was double-quoting the expressions. Here are the other possibilities:

The parentheses don't do anything, so another way of rewriting is:

let a="$b * $c"

Bash numeric evaluation will dereference variables for you, so this does the same thing:

bash
let a="b * c"


I think that last one may be harder to read, since everywhere else variable dereferences are written `$foo`.

Finally, the really bash-y way of writing this is

``` bash
a=$(( b * c ))

@gfoury
Copy link
Contributor Author

gfoury commented Feb 24, 2016

I added a makefile, with an easy way to tell Shellcheck to _SHUT UP_ about certain warnings. :-)

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.

1 participant