From 2077455498aa63d8e0d16986608a53ad9570b00d Mon Sep 17 00:00:00 2001 From: Robert Kirkman Date: Mon, 23 Jun 2025 10:26:38 -0500 Subject: [PATCH] tree-wide: code style and print message cleanup - Continuation of https://github.com/termux/termux-tools/pull/179, particularly these topics from there: - https://github.com/termux/termux-tools/pull/179#pullrequestreview-2908531632 - https://github.com/termux/termux-tools/pull/179#discussion_r2160055157 - Contains things that were proposed there, but that it was decided to separate out into another PR for organization and easier review - Replaces backtick command substitution syntax with dollar symbol and parentheses command substitution syntax in shell scripts - Places double quote symbols around all string (non-numerical) variable references that aren't possibly-empty references in the argument lists of command invocations - (placing double quote symbols around a possibly-empty variable in a command invocation is a case where the double quote symbols would cause an error to happen) - Places double quote symbols around all instances of `@TERMUX_PREFIX@` and other replaced strings in `.sh.in` files - Remove unused function `hostname()` from `pkg` - Remove unused variable `PREFIX` from `termux-restore` - Unify and clarify instances of `login`-related error message in `chsh` --- scripts/chsh.in | 16 ++++++---------- scripts/dalvikvm.in | 6 +++--- scripts/login.in | 12 ++++++------ scripts/pkg.in | 6 +----- scripts/su.in | 4 ++-- scripts/termux-backup.in | 2 +- scripts/termux-change-repo.in | 28 ++++++++++++++-------------- scripts/termux-open.in | 10 +++++----- scripts/termux-restore.in | 4 +--- 9 files changed, 39 insertions(+), 49 deletions(-) diff --git a/scripts/chsh.in b/scripts/chsh.in index 32702b07..1c8ef6bb 100644 --- a/scripts/chsh.in +++ b/scripts/chsh.in @@ -11,11 +11,7 @@ show_usage() { echo "usage: chsh [-s shell]" echo "Change the login shell." echo - if [ "${1:-}" = "login" ]; then - echo "The shell value must be one of following and cannot be 'login':" - else - echo "The shell value must be one of following:" - fi + echo "The shell value must be one of following and cannot be 'login':" echo " - Empty value to restore default shell." echo " - Absolute path to shell starting with a '/'." echo " - Relative path to shell not starting with a '/' relative to '@TERMUX_PREFIX@/bin'." @@ -28,10 +24,6 @@ set_shell() { exit 0 fi - if [ "$1" = "login" ]; then - show_usage "login" - exit 1 - fi mkdir -p "$HOME/.termux" unset NEW_SHELL @@ -44,18 +36,22 @@ set_shell() { if ! is_executable_file "$SHELL_TARGET"; then echo "The shell file '$SHELL_TARGET' is not an executable file." 1>&2 + echo + show_usage exit 1 fi if [ "$SHELL_TARGET" -ef "@TERMUX_PREFIX@/bin/login" ]; then echo "The shell file '$SHELL_TARGET' must not point to the '@TERMUX_PREFIX@/bin/login' script." 1>&2 + echo + show_usage exit 1 fi ln -sf "$SHELL_TARGET" "$HOME/.termux/shell" } -O=`getopt -l help -- hs: "$@"` +O="$(getopt -l help -- hs: "$@")" eval set -- "$O" while true; do case "$1" in diff --git a/scripts/dalvikvm.in b/scripts/dalvikvm.in index d36df551..326fcd5e 100644 --- a/scripts/dalvikvm.in +++ b/scripts/dalvikvm.in @@ -1,8 +1,8 @@ #!/bin/sh # There needs to be a folder at $ANDROID_DATA/dalvik-cache -export ANDROID_DATA=@TERMUX_PREFIX@/var/android/ -mkdir -p $ANDROID_DATA/dalvik-cache +export ANDROID_DATA="@TERMUX_PREFIX@/var/android/" +mkdir -p "$ANDROID_DATA/dalvik-cache" if [ -x /apex/com.android.art/bin/dalvikvm ]; then DALVIKVM="/apex/com.android.art/bin/dalvikvm" @@ -11,4 +11,4 @@ else fi unset LD_LIBRARY_PATH LD_PRELOAD -exec "$DALVIKVM" -Xusejit:true -Xnoimage-dex2oat -Djava.io.tmpdir=@TERMUX_PREFIX@/tmp "$@" +exec "$DALVIKVM" -Xusejit:true -Xnoimage-dex2oat -Djava.io.tmpdir="@TERMUX_PREFIX@/tmp" "$@" diff --git a/scripts/login.in b/scripts/login.in index e8620898..461fb7fe 100644 --- a/scripts/login.in +++ b/scripts/login.in @@ -20,8 +20,8 @@ if tty >/dev/null 2>&1 && [ $# = 0 ] && [ ! -f ~/.hushlogin ] && [ -z "$TERMUX_H [ ! -x ~/.termux/motd.sh ] && chmod u+x ~/.termux/motd.sh ~/.termux/motd.sh # Default to termux-tools package provided static motd file if it exists - elif [ -f @TERMUX_PREFIX@/etc/motd ]; then - cat @TERMUX_PREFIX@/etc/motd + elif [ -f "@TERMUX_PREFIX@/etc/motd" ]; then + cat "@TERMUX_PREFIX@/etc/motd" fi else # This variable shouldn't be kept set. @@ -29,8 +29,8 @@ else fi # TERMUX_VERSION env variable has been exported since v0.107 and PATH was being set to following value in <0.104. Last playstore version was v0.101. -if tty >/dev/null 2>&1 && [ $# = 0 ] && [ -f @TERMUX_PREFIX@/etc/motd-playstore ] && [ -z "$TERMUX_VERSION" ] && [ "$PATH" = "@TERMUX_PREFIX@/bin:@TERMUX_PREFIX@/bin/applets" ]; then - printf '\033[0;31m'; cat @TERMUX_PREFIX@/etc/motd-playstore; printf '\033[0m' +if tty >/dev/null 2>&1 && [ $# = 0 ] && [ -f "@TERMUX_PREFIX@/etc/motd-playstore" ] && [ -z "$TERMUX_VERSION" ] && [ "$PATH" = "@TERMUX_PREFIX@/bin:@TERMUX_PREFIX@/bin/applets" ]; then + printf '\033[0;31m'; cat "@TERMUX_PREFIX@/etc/motd-playstore"; printf '\033[0m' fi unset SHELL @@ -74,8 +74,8 @@ elif [ -f "@TERMUX_PREFIX@/lib/libtermux-exec.so" ]; then $SHELL -c "coreutils --coreutils-prog=true" > /dev/null 2>&1 || unset LD_PRELOAD fi -if [ -f @TERMUX_PREFIX@/etc/termux-login.sh ]; then - . @TERMUX_PREFIX@/etc/termux-login.sh +if [ -f "@TERMUX_PREFIX@/etc/termux-login.sh" ]; then + . "@TERMUX_PREFIX@/etc/termux-login.sh" fi if [ -n "$TERM" ]; then diff --git a/scripts/pkg.in b/scripts/pkg.in index 8fc9ea8a..9d7b2ede 100644 --- a/scripts/pkg.in +++ b/scripts/pkg.in @@ -21,7 +21,7 @@ show_help() { elif [ "$TERMUX_APP_PACKAGE_MANAGER" = "pacman" ]; then cache_dir="@TERMUX_PREFIX@/var/cache/pacman/pkg" fi - cache_size=$(du -sh "$cache_dir" 2>/dev/null | cut -f1) + cache_size="$(du -sh "$cache_dir" 2>/dev/null | cut -f1)" echo 'Usage: pkg [--check-mirror] command [arguments]' echo @@ -96,10 +96,6 @@ check_command() { fi } -hostname() { - echo "$1" | awk -F'[/:]' '{print $4}' -} - last_modified() { local mtime local now diff --git a/scripts/su.in b/scripts/su.in index 3d173ba9..76c46f16 100644 --- a/scripts/su.in +++ b/scripts/su.in @@ -4,10 +4,10 @@ unset LD_LIBRARY_PATH LD_PRELOAD for p in /debug_ramdisk/su /sbin/su /system/sbin/su /system/bin/su /system/xbin/su /su/bin/su /magisk/.core/bin/su do - if [ -x $p ]; then + if [ -x "$p" ]; then # The su tool may require programs in PATH: PATH=/debug_ramdisk:/sbin:/sbin/su:/su/bin:/su/xbin:/system/bin:/system/xbin \ - exec $p "$@" + exec "$p" "$@" fi done diff --git a/scripts/termux-backup.in b/scripts/termux-backup.in index 4c05bb41..b5d8485a 100644 --- a/scripts/termux-backup.in +++ b/scripts/termux-backup.in @@ -31,7 +31,7 @@ set -e -u -export PREFIX=@TERMUX_PREFIX@ +export PREFIX="@TERMUX_PREFIX@" msg() { echo "$*" >&2 diff --git a/scripts/termux-change-repo.in b/scripts/termux-change-repo.in index 666d4405..e92ef938 100644 --- a/scripts/termux-change-repo.in +++ b/scripts/termux-change-repo.in @@ -30,7 +30,7 @@ select_repository_group() { MIRRORS+=("Mirrors in Oceania" "All in Oceania") MIRRORS+=("Mirrors in Russia" "All in Russia") - local TEMPFILE="$(mktemp @TERMUX_PREFIX@/tmp/mirror.XXXXXX)" + local TEMPFILE="$(mktemp "@TERMUX_PREFIX@/tmp/mirror.XXXXXX")" dialog \ --title "termux-change-repo" --clear \ --menu "Which group of mirrors do you want to use?" 0 0 0 \ @@ -55,31 +55,31 @@ select_repository_group() { if [ "$mirror_group" == "Mirrors in Asia" ]; then echo "[*] Mirrors in Asia (excl. Chinese Mainland and Russia) selected" - unlink_and_link ${MIRROR_BASE_DIR}/asia + unlink_and_link "${MIRROR_BASE_DIR}/asia" elif [ "$mirror_group" == "Mirrors in Chinese Mainland" ]; then echo "[*] Mirrors in Chinese Mainland selected" - unlink_and_link ${MIRROR_BASE_DIR}/chinese_mainland + unlink_and_link "${MIRROR_BASE_DIR}/chinese_mainland" elif [ "$mirror_group" == "Mirrors in Europe" ]; then echo "[*] Mirrors in Europe selected" - unlink_and_link ${MIRROR_BASE_DIR}/europe + unlink_and_link "${MIRROR_BASE_DIR}/europe" elif [ "$mirror_group" == "Mirrors in North America" ]; then echo "[*] Mirrors in North America selected" - unlink_and_link ${MIRROR_BASE_DIR}/north_america + unlink_and_link "${MIRROR_BASE_DIR}/north_america" elif [ "$mirror_group" == "Mirrors in Oceania" ]; then echo "[*] Mirrors in Oceania selected" - unlink_and_link ${MIRROR_BASE_DIR}/oceania + unlink_and_link "${MIRROR_BASE_DIR}/oceania" elif [ "$mirror_group" == "Mirrors in Russia" ]; then echo "[*] Mirrors in Russia selected" - unlink_and_link ${MIRROR_BASE_DIR}/russia + unlink_and_link "${MIRROR_BASE_DIR}/russia" elif [ "$mirror_group" == "All mirrors" ]; then echo "[*] All mirrors selected" - unlink_and_link ${MIRROR_BASE_DIR}/all + unlink_and_link "${MIRROR_BASE_DIR}/all" else echo "[!] Error: unknown mirror group: '$1'. Exiting" @@ -102,7 +102,7 @@ get_mirror_description() { } select_individual_mirror() { - mirrors=($(find ${MIRROR_BASE_DIR}/{asia,chinese_mainland,europe,north_america,oceania,russia}/ -type f ! -name "*\.dpkg-old" ! -name "*\.dpkg-new" ! -name "*~")) + mirrors=($(find "${MIRROR_BASE_DIR}"/{asia,chinese_mainland,europe,north_america,oceania,russia}/ -type f ! -name "*\.dpkg-old" ! -name "*\.dpkg-new" ! -name "*~")) # Choose default mirror per default MIRRORS=("$(get_mirror_url "${MIRROR_BASE_DIR}/default")" "$(get_mirror_description "${MIRROR_BASE_DIR}/default")") @@ -114,7 +114,7 @@ select_individual_mirror() { MIRRORS+=("$mirror_url" "$(get_mirror_description "$mirror")") done - local TEMPFILE="$(mktemp @TERMUX_PREFIX@/tmp/mirror.XXXXXX)" + local TEMPFILE="$(mktemp "@TERMUX_PREFIX@/tmp/mirror.XXXXXX")" dialog \ --title "termux-change-repo" --clear \ --menu "Which mirror do you want to use?" 0 0 0 \ @@ -137,7 +137,7 @@ select_individual_mirror() { mirror="$(cat "$TEMPFILE")" echo "[*] Mirror $(get_mirror_url "$mirror") selected" - unlink_and_link "$(find ${MIRROR_BASE_DIR} -name $mirror)" + unlink_and_link "$(find "${MIRROR_BASE_DIR}" -name "$mirror")" rm "$TEMPFILE" } @@ -160,12 +160,12 @@ fi if [ "$TERMUX_APP_PACKAGE_MANAGER" = "pacman" ]; then read -p "Warning: termux-change-repo can only change mirrors for apt, is that what you intended? [y|n] " -n 1 -r echo - [[ ${REPLY} =~ ^[Nn]$ ]] && exit + [[ "${REPLY}" =~ ^[Nn]$ ]] && exit fi mkdir -p "@TERMUX_PREFIX@/tmp" || exit $? -TEMPFILE="$(mktemp @TERMUX_PREFIX@/tmp/termux-change-repo.XXXXXX)" +TEMPFILE="$(mktemp "@TERMUX_PREFIX@/tmp/termux-change-repo.XXXXXX")" MODES=() MODES+=("Mirror group" "Rotate between several mirrors (recommended)") @@ -180,7 +180,7 @@ clear case $retval in 0) - case "$(cat $TEMPFILE)" in + case "$(cat "$TEMPFILE")" in "Mirror group") select_repository_group ;; diff --git a/scripts/termux-open.in b/scripts/termux-open.in index 541b797e..161b66a5 100644 --- a/scripts/termux-open.in +++ b/scripts/termux-open.in @@ -12,11 +12,11 @@ show_usage () { exit 0 } -TEMP=`getopt \ - -n $SCRIPTNAME \ - -o h \ - --long send,view,chooser,content-type:,help\ - -- "$@"` +TEMP="$(getopt \ + -n "$SCRIPTNAME" \ + -o h \ + --long send,view,chooser,content-type:,help \ + -- "$@")" eval set -- "$TEMP" ACTION=android.intent.action.VIEW diff --git a/scripts/termux-restore.in b/scripts/termux-restore.in index 39efa467..fb905e23 100644 --- a/scripts/termux-restore.in +++ b/scripts/termux-restore.in @@ -36,8 +36,6 @@ set -e -u -export PREFIX=@TERMUX_PREFIX@ - msg() { echo "$*" >&2 } @@ -80,7 +78,7 @@ fi case "$1" in -\?|-h|--help|--usage) show_usage; exit 0;; - *) BACKUP_FILE_PATH=$1;; + *) BACKUP_FILE_PATH="$1";; esac if [ "$BACKUP_FILE_PATH" != "-" ] && [ ! -e "$BACKUP_FILE_PATH" ]; then