ctubbsii commented on code in PR #2224:
URL: https://github.com/apache/zookeeper/pull/2224#discussion_r1970547493
##########
bin/zkCli.sh:
##########
@@ -27,17 +30,23 @@
# use POSIX interface, symlink is followed automatically
ZOOBIN="${BASH_SOURCE-$0}"
-ZOOBIN="$(dirname "${ZOOBIN}")"
-ZOOBINDIR="$(cd "${ZOOBIN}"; pwd)"
+ZOOBIN="$(dirname "$ZOOBIN")"
+ZOOBINDIR="$(cd "$ZOOBIN" && pwd)"
-if [ -e "$ZOOBIN/../libexec/zkEnv.sh" ]; then
+if [[ -e "$ZOOBIN/../libexec/zkEnv.sh" ]]; then
+ # shellcheck source=bin/zkEnv.sh
. "$ZOOBINDIR"/../libexec/zkEnv.sh
else
+ # shellcheck source=bin/zkEnv.sh
. "$ZOOBINDIR"/zkEnv.sh
fi
ZOO_LOG_FILE=zookeeper-$USER-cli-$HOSTNAME.log
-"$JAVA" "-Dzookeeper.log.dir=${ZOO_LOG_DIR}"
"-Dzookeeper.log.file=${ZOO_LOG_FILE}" \
- -cp "$CLASSPATH" $CLIENT_JVMFLAGS $JVMFLAGS \
- org.apache.zookeeper.ZooKeeperMain "$@"
+# shellcheck disable=SC2206
+clientflags=($CLIENT_JVMFLAGS)
+# shellcheck disable=SC2206
+flags=($JVMFLAGS)
+"$JAVA" "-Dzookeeper.log.dir=$ZOO_LOG_DIR"
"-Dzookeeper.log.file=$ZOO_LOG_FILE" \
+ "${clientflags[@]}" "${flags[@]}" \
+ org.apache.zookeeper.ZooKeeperMain "$@"
Review Comment:
This makes it clear that the intent is to treat the `$JVMFLAGS` and
`$CLIENT_JVMFLAGS` scalar variables as whitespace-delimited arrays. shellcheck
warns about this because it does not know the author's intent. In order to
suppress that shellcheck warning, I made the parsing of the scalar as an array
an explicit separate action. shellcheck will still warn about that, but I
suppress that with a very narrow exception (see lines 46 and 48).
It is possible to suppress the warning without changing the line....
however, if you do that, the suppression would be far too broad, affecting all
the parameters on the java command, and it would hide future programming errors
that shellcheck would normally catch. The way I wrote it here keeps a very
narrow suppression, so we don't hide future programming errors, but without
changing any behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
