mscs
mscs copied to clipboard
[WIP] Fixshellcheck
This is a work in progress to fix all of the shellcheck errors/warnings/etc.
I've cleaned up some of the issues found by shellcheck. There are a bunch more!
My tests (up to commit cfe1c565158138cf59d02286e64450fdaf7328e6) show that everything seems to be working still, but this is going to need more testing..
I merged the changes that I had been working on but had not yet committed. Things seem to be working, but we need to do some serious testing with all of this...
We have a few more issues to tackle, but with these commits, we are getting close.
A lot of the SC2181
errors can be fixed by changing the way we do error output, I think. For instance, we do something like below to propagate error messages to the user:
getCurrentMinecraftVersion() {
[...]
# Print an error and exit if the version string is empty.
if [ -z "$VERSION" ]; then
printf "Error detecting the current Minecraft version.\n"
exit 1
fi
printf "%s" "$VERSION"
}
getClientVersion() {
CURRENT_VERSION=$(getCurrentMinecraftVersion "$1")
if [ $? -ne 0 ]; then
printf "%s\n" "$CURRENT_VERSION"
exit 1
fi
[...]
}
If we instead pipe these errors to STDERR, I think it would simplify the code:
getCurrentMinecraftVersion() {
[...]
# Print an error and exit if the version string is empty.
if [ -z "$VERSION" ]; then
printf "Error detecting the current Minecraft version.\n" 1>&2
exit 1
fi
printf "%s" "$VERSION"
}
getClientVersion() {
CURRENT_VERSION=$(getCurrentMinecraftVersion "$1")
[ -n "$CURRENT_VERSION" ] || exit 1
[...]
}
This is untested...
I got rid of some of the SC2181
errors by doing what I mentioned above. It seemed to work.
Removing LOCAL broke a lot of things. I think I have a workaround...
So removing LOCAL made all variables global. This caused subroutines to overwrite global variables. A POSIX compatible way to fix this was to simply start a new shell for each subroutine by swapping curly brackets with parens. See commit 5c81c6d9b4998c3b0b93083d66d9d855a3876089 for details.
With this change, I think we are closer to having a working script again. Please test this branch to see if we are ready to merge it into the main branch.
We might need a rebase on the current main
branch at this point in time.
I'm tempted to NACK this pull request and start from scratch. I just set the shellcheck severity to "error" and fixed the few issues identified, so the main branch is now reporting no errors.