Run shellcheck on all branches, fix all violations, and update documentation (#31)

* Initial plan

* Update GitHub Action to run shellcheck on every commit to all branches

Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>

* Fix: Prevent duplicate workflow runs on PRs

Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>

* Fix all shellcheck violations across the codebase

Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>

* Update AGENTS.md to document new shellcheck and quoting requirements

Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>

* Fix whiptail checklist showing usage text instead of TUI dialog

Remove quotes from conditional --scrolltext argument to prevent
empty string being passed as an argument when RECMD_SCROLL is false.
Added shellcheck directives to document intentional word splitting.

Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>

Author: Copilot

.github/workflows/analyze.yml

@@ -6,7 +6,6 @@ on:
   push:
     branches: [ "master" ]
   pull_request:
-    branches: [ "master" ]
 
   workflow_dispatch:
 

AGENTS.md

@@ -37,9 +37,12 @@ The repository is organized as follows:
 * Document all functions with clear descriptions of parameters and behavior.
 * Use consistent indentation (2 spaces).
 * Always quote variable expansions unless you specifically need word splitting.
+* Always quote command substitutions: `"$(command)"` not `$(command)`.
 * Use `[[ ]]` for pattern matching (wildcards, regex) and bash-specific features (case conversion).
 * Use `[ ]` for basic POSIX-compatible tests (string equality, -z, -n, numeric comparisons).
 * Use `command -v` instead of `which` for checking command existence.
+* Separate variable declarations from assignments that run commands: `local var; var=$(command)` instead of `local var=$(command)`.
+* Use shellcheck directives instead of exporting variables to avoid polluting the environment.
 
 ### Running Quality Checks Locally
 
@@ -49,19 +52,14 @@ Before committing code, developers should run:
 # Restore dependencies (ubuntu/debian syntax)
 sudo apt install shellcheck
 
-# Run the check on all scripts
-shellcheck script-dialog.sh
-shellcheck test.sh
-shellcheck screenshot-dialogs.sh
-
-# Or check all at once
-shellcheck ./*.sh -x
+# Run the check on all scripts (including cross-file sourcing)
+shellcheck ./*.sh extras/*.sh -x
 
 # Run integration tests (requires a terminal or GUI environment)
 bash test.sh
 ```
 
-And correct any remaining issues reported.
+All code must pass shellcheck with zero violations before committing.
 
 ## Bash-Specific Patterns and Best Practices
 
@@ -115,6 +113,35 @@ The library detects platforms and desktop environments:
 - Windows/WSL: `[[ $OSTYPE == msys ]] || [[ $(uname -r | tr '[:upper:]' '[:lower:]') == *wsl* ]]` (pattern matching)
 - Linux desktops: Detected via `$XDG_CURRENT_DESKTOP`, `$XDG_SESSION_DESKTOP`, or running processes via `pgrep -l "process-name"` (gnome-shell, mutter, kwin)
 
+### Shellcheck Directives and Cross-File Variables
+
+The library uses a modular structure where `init.sh` defines variables used across multiple files. To handle shellcheck warnings about these cross-file variables:
+
+1. **For variables defined in init.sh and used elsewhere**:
+   - Add `# shellcheck disable=SC2154` at the top of files that use variables from init.sh
+   - This suppresses "variable is referenced but not assigned" warnings
+
+2. **For variables exposed to library users**:
+   - Add `# shellcheck disable=SC2034` at the file level in init.sh
+   - This suppresses "variable appears unused" warnings for intentionally exposed variables
+   - Do NOT export these variables to avoid polluting the user's environment
+
+3. **For intentional command word splitting**:
+   - Quote command substitutions: `"$([ "$RECMD_SCROLL" == true ] && echo "--scrolltext")"`
+   - This prevents SC2046 warnings while maintaining correct behavior
+
+Example:
+```bash
+# At the top of a file using init.sh variables
+#!/usr/bin/env bash
+# Multi-UI Scripting - Helper Functions
+
+# Variables set in init.sh and used here
+# shellcheck disable=SC2154
+
+# Now you can use variables like $bold, $normal, $red without warnings
+```
+
 ## Cross-Platform Considerations
 
 ### Platform-Specific Behavior
@@ -164,14 +191,22 @@ Use `screenshot-dialogs.sh` to capture visual evidence of changes:
 
 ### Automated Testing (CI)
 
-The GitHub Actions workflow runs shellcheck on all `.sh` files. All code must pass shellcheck without errors.
+The GitHub Actions workflow runs shellcheck on all `.sh` files for every PR and direct commit to master. All code must pass shellcheck without errors or warnings.
+
+The workflow is configured to:
+- Run on all pull requests (via `pull_request` trigger)
+- Run on direct commits to `master` branch (via `push` trigger)
+- Avoid duplicate runs on PRs by not triggering `push` for non-master branches
 
 ## Common Patterns and Pitfalls to Avoid
 
 ### DO:
 * Capture exit status immediately after the command that generates it
 * Use `|| exit "$?"` after command substitutions that might be cancelled
 * Quote variable expansions: `"$variable"` not `$variable`
+* Quote command substitutions: `"$(command)"` not `$(command)`
+* Separate variable declarations from assignments: `local var; var=$(cmd)` not `local var=$(cmd)`
+* Use shellcheck directives for cross-file variables instead of exporting them
 * Test with multiple interfaces (zenity, kdialog, whiptail, dialog)
 * Document why you're disabling shellcheck rules if necessary
 * Consider both GUI and TUI behavior when implementing features
@@ -184,6 +219,8 @@ The GitHub Actions workflow runs shellcheck on all `.sh` files. All code must pa
 * Remove or modify cancel detection without thorough testing
 * Break backward compatibility with existing scripts
 * Copy complex patterns from other projects without understanding them
+* Export variables unnecessarily - use shellcheck directives instead to avoid environment pollution
+* Leave unquoted variables or command substitutions that trigger shellcheck warnings
 
 ## Recent Learnings from PR Feedback
 
@@ -202,6 +239,21 @@ From PR #28 (Add configurable exit on dialog cancellation):
    - It follows the `timeout` command convention (which also uses 124)
    - It's less generic than 1 or 2
 
+From PR (Shellcheck compliance and CI improvements):
+
+1. **Quoting is mandatory**: All variable expansions and command substitutions must be quoted to pass shellcheck. This prevents word splitting and globbing issues.
+
+2. **Separate declarations from assignments**: Using `local var=$(command)` masks the command's exit status. Always separate them: `local var; var=$(command)` followed by `exit_status=$?`.
+
+3. **Use shellcheck directives, not exports**: When variables are defined in `init.sh` and used in other sourced files:
+   - Add `# shellcheck disable=SC2154` at the top of files using those variables
+   - Add `# shellcheck disable=SC2034` in init.sh for variables exposed to library users
+   - Do NOT export variables just to satisfy shellcheck - this pollutes the user's environment
+
+4. **CI must catch issues early**: The GitHub Actions workflow now runs on all PRs and master commits, ensuring code quality before merge.
+
+5. **Zero tolerance for shellcheck violations**: All code must pass `shellcheck ./*.sh extras/*.sh -x` with zero warnings or errors.
+
 ## Boundaries and Guardrails
 
 * **NEVER** add a feature only supported on one OS or desktop environment.

datepicker.sh

@@ -3,6 +3,9 @@
 # https://github.com/lunarcloud/script-dialog
 # LGPL-2.1 license
 
+# Variables set in init.sh and used here
+# shellcheck disable=SC2154
+
 #######################################
 # Display a calendar date selector dialog
 # GLOBALS:
@@ -42,13 +45,14 @@ function datepicker() {
 
   local exit_status=0
   if [ "$INTERFACE" == "whiptail" ]; then
+    # shellcheck disable=SC2034  # SYMBOL used by whiptail in this context
     local SYMBOL=$CALENDAR_SYMBOL
     STANDARD_DATE=$(inputbox "Input Date (DD/MM/YYYY)" "$NOW")
   elif [ "$INTERFACE" == "dialog" ]; then
     STANDARD_DATE=$(dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" --stdout --calendar "${CALENDAR_SYMBOL}Choose Date" 0 40)
     exit_status=$?
   elif [ "$INTERFACE" == "zenity" ]; then
-    INPUT_DATE=$(zenity --title="$GUI_TITLE" $ZENITY_ICON_ARG "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --calendar "Select Date")
+    INPUT_DATE=$(zenity --title="$GUI_TITLE" "$ZENITY_ICON_ARG" "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --calendar "Select Date")
     exit_status=$?
     MONTH=$(echo "$INPUT_DATE" | cut -d'/' -f1)
     DAY=$(echo "$INPUT_DATE" | cut -d'/' -f2)

helpers.sh

@@ -3,6 +3,9 @@
 # https://github.com/lunarcloud/script-dialog
 # LGPL-2.1 license
 
+# Variables set in init.sh and used here
+# shellcheck disable=SC2154
+
 #######################################
 # Attempts to run a privileged command (sudo or equivalent)
 # GLOBALS:
@@ -17,12 +20,12 @@
 # 	0 if success, non-zero otherwise.
 #######################################
 function superuser() {
-  if [ $NO_SUDO == true ]; then
+  if [ "$NO_SUDO" == true ]; then
     (>&2 echo "${red}No sudo available!${normal}")
     return 201
   fi
 
-  if [ $SUDO_USE_INTERFACE == true ]; then
+  if [ "$SUDO_USE_INTERFACE" == true ]; then
     ACTIVITY="Enter password to run \"$*\""
     password "$@" | sudo -p "" -S -- "$@"
   elif [[ "$SUDO" == *"pkexec"* ]]; then
@@ -50,8 +53,10 @@ function superuser() {
 #######################################
 function _calculate-gui-title() {
   if [ -n "$ACTIVITY" ]; then
+    # shellcheck disable=SC2034  # GUI_TITLE is used by other functions
     GUI_TITLE="$ACTIVITY - $APP_NAME"
   else
+    # shellcheck disable=SC2034
     GUI_TITLE="$APP_NAME"
   fi
 }
@@ -110,7 +115,9 @@ function _calculate-tui-size() {
 
   # Handle empty string case
   if [ -z "$TEST_STRING" ]; then
+    # shellcheck disable=SC2153  # MIN_COLS and MIN_LINES are defined in init.sh
     RECMD_COLS=$MIN_COLS
+    # shellcheck disable=SC2153
     RECMD_LINES=$MIN_LINES
     return
   fi
@@ -166,10 +173,12 @@ function _calculate-tui-size() {
   # Enforce maximum constraints
   if [ "$RECMD_LINES" -gt "$MAX_LINES" ] ; then
     RECMD_LINES=$MAX_LINES
+    # shellcheck disable=SC2034  # RECMD_SCROLL is used by dialog functions
     RECMD_SCROLL=true
   fi
   if [ "$RECMD_COLS" -gt "$MAX_COLS" ]; then
     RECMD_COLS=$MAX_COLS
+    # shellcheck disable=SC2034
     RECMD_SCROLL=true
   fi
 

init.sh

@@ -3,6 +3,9 @@
 # https://github.com/lunarcloud/script-dialog
 # LGPL-2.1 license
 
+# Disable SC2034 for the entire file as variables defined here are used by scripts that source this library
+# shellcheck disable=SC2034
+
 # Disable this rule, as it interferes with purely-numeric parameters
 # shellcheck disable=SC2046
 
@@ -158,6 +161,7 @@ fi
 # Variables
 ################################
 
+# These variables are intentionally exposed for use by scripts that source this library
 APP_NAME="Script"
 ACTIVITY=""
 GUI_TITLE="$APP_NAME"

inputs.sh

@@ -3,6 +3,9 @@
 # https://github.com/lunarcloud/script-dialog
 # LGPL-2.1 license
 
+# Variables set in init.sh and used here
+# shellcheck disable=SC2154
+
 #######################################
 # Display a text input box
 # GLOBALS:
@@ -48,7 +51,7 @@ function inputbox() {
     INPUT=$(dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" --inputbox "${SYMBOL} $1" "$RECMD_LINES" "$RECMD_COLS" "$2" 3>&1 1>&2 2>&3)
     exit_status=$?
   elif [ "$INTERFACE" == "zenity" ]; then
-    INPUT="$(zenity --entry --title="$GUI_TITLE" $ZENITY_ICON_ARG "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --text="$1" --entry-text "$2")"
+    INPUT="$(zenity --entry --title="$GUI_TITLE" "$ZENITY_ICON_ARG" "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --text="$1" --entry-text "$2")"
     exit_status=$?
   elif [ "$INTERFACE" == "kdialog" ]; then
     INPUT="$(kdialog --title "$GUI_TITLE" --icon "$GUI_ICON" --inputbox "$1" "$2")"
@@ -119,7 +122,7 @@ function userandpassword() {
   elif [ "$INTERFACE" == "dialog" ]; then
     mapfile -t CREDS < <( dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" --insecure --mixedform "Login:" "$RECMD_LINES" "$RECMD_COLS" 0 "Username: " 1 1 "$SUGGESTED_USERNAME" 1 11 22 0 0 "Password :" 2 1 "" 2 11 22 0 1 3>&1 1>&2 2>&3 )
   elif [ "$INTERFACE" == "zenity" ]; then
-    ENTRY=$(zenity --title="$GUI_TITLE" $ZENITY_ICON_ARG "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --password --username "$SUGGESTED_USERNAME")
+    ENTRY=$(zenity --title="$GUI_TITLE" "$ZENITY_ICON_ARG" "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --password --username "$SUGGESTED_USERNAME")
     local exit_status=$?
     CREDS[0]=$(echo "$ENTRY" | cut -d'|' -f1)
     CREDS[1]=$(echo "$ENTRY" | cut -d'|' -f2)
@@ -182,6 +185,7 @@ function password() {
     GUI_ICON=$XDG_ICO_PASSWORD
   fi
   _calculate-gui-title
+  # shellcheck disable=SC2034  # TEST_STRING is used by _calculate-tui-rows-cols
   TEST_STRING="${PASSWORD_SYMBOL}$1"
   _calculate-tui-size
 
@@ -193,7 +197,7 @@ function password() {
     PASSWORD=$(dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY"  --passwordbox "$1" "$RECMD_LINES" "$RECMD_COLS" 3>&1 1>&2 2>&3)
     exit_status=$?
   elif [ "$INTERFACE" == "zenity" ]; then
-    PASSWORD=$(zenity --title="$GUI_TITLE" $ZENITY_ICON_ARG "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --password)
+    PASSWORD=$(zenity --title="$GUI_TITLE" "$ZENITY_ICON_ARG" "$GUI_ICON" ${ZENITY_HEIGHT+--height=$ZENITY_HEIGHT} ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --password)
     exit_status=$?
   elif [ "$INTERFACE" == "kdialog" ]; then
     PASSWORD=$(kdialog --title="$GUI_TITLE" --icon "$GUI_ICON" --password "$1")

lists.sh

@@ -3,6 +3,10 @@
 # https://github.com/lunarcloud/script-dialog
 # LGPL-2.1 license
 
+# Variables set in init.sh and used here
+# shellcheck disable=SC2034  # line variable in read loops
+# shellcheck disable=SC2154
+
 #######################################
 # Display a list of multiply-selectable items
 # GLOBALS:
@@ -67,10 +71,13 @@ function checklist() {
 
   local exit_status=0
   if [ "$INTERFACE" == "whiptail" ]; then
-    mapfile -t CHOSEN_ITEMS < <( whiptail --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --checklist "${QUESTION_SYMBOL}$TEXT" $RECMD_LINES $RECMD_COLS "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
+    # shellcheck disable=SC2046  # Intentional word splitting for conditional argument
+    mapfile -t CHOSEN_ITEMS < <( whiptail --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --checklist "${QUESTION_SYMBOL}$TEXT" "$RECMD_LINES" "$RECMD_COLS" "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
     exit_status=$?
   elif [ "$INTERFACE" == "dialog" ]; then
-    local DIALOG_OUTPUT=$(dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --separate-output --checklist "${QUESTION_SYMBOL}$TEXT" $RECMD_LINES $RECMD_COLS "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
+    local DIALOG_OUTPUT
+    # shellcheck disable=SC2046  # Intentional word splitting for conditional argument
+    DIALOG_OUTPUT=$(dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --separate-output --checklist "${QUESTION_SYMBOL}$TEXT" "$RECMD_LINES" "$RECMD_COLS" "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
     exit_status=$?
     IFS=$'\n' read -r -d '' -a CHOSEN_LIST < <( echo "${DIALOG_OUTPUT[@]}" )
 
@@ -94,9 +101,10 @@ function checklist() {
       shift
       shift
     done
-    local ZENITY_OUTPUT=$(zenity --title "$GUI_TITLE" $ZENITY_ICON_ARG "$GUI_ICON" --height="${ZENITY_HEIGHT-512}" ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --list --text "$TEXT" --checklist --column "" --column "Value" --column "Description" "${OPTIONS[@]}")
+    local ZENITY_OUTPUT
+    ZENITY_OUTPUT=$(zenity --title "$GUI_TITLE" "$ZENITY_ICON_ARG" "$GUI_ICON" --height="${ZENITY_HEIGHT-512}" ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --list --text "$TEXT" --checklist --column "" --column "Value" --column "Description" "${OPTIONS[@]}")
     exit_status=$?
-    IFS=$'|' read -r -d '' -a CHOSEN_LIST < <( echo $ZENITY_OUTPUT )
+    IFS=$'|' read -r -d '' -a CHOSEN_LIST < <( echo "$ZENITY_OUTPUT" )
 
     local CHOSEN_ITEMS=()
     for value in "${CHOSEN_LIST[@]}"
@@ -195,14 +203,16 @@ function radiolist() {
 
   local exit_status=0
   if [ "$INTERFACE" == "whiptail" ]; then
-    CHOSEN_ITEM=$( whiptail --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --radiolist "${QUESTION_SYMBOL}$TEXT" $RECMD_LINES $RECMD_COLS "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
+    # shellcheck disable=SC2046  # Intentional word splitting for conditional argument
+    CHOSEN_ITEM=$( whiptail --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --radiolist "${QUESTION_SYMBOL}$TEXT" "$RECMD_LINES" "$RECMD_COLS" "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
     exit_status=$?
     # For TUI interfaces, empty response indicates cancel
     if [ $exit_status -ne 0 ] || [[ -z "$CHOSEN_ITEM" ]]; then
       exit "$SCRIPT_DIALOG_CANCEL_EXIT_CODE"
     fi
   elif [ "$INTERFACE" == "dialog" ]; then
-    CHOSEN_ITEM=$( dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --radiolist "${QUESTION_SYMBOL}$TEXT" $RECMD_LINES $RECMD_COLS "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
+    # shellcheck disable=SC2046  # Intentional word splitting for conditional argument
+    CHOSEN_ITEM=$( dialog --clear --backtitle "$APP_NAME" --title "$ACTIVITY" $([ "$RECMD_SCROLL" == true ] && echo "--scrolltext") --radiolist "${QUESTION_SYMBOL}$TEXT" "$RECMD_LINES" "$RECMD_COLS" "$NUM_OPTIONS" "$@"  3>&1 1>&2 2>&3)
     exit_status=$?
     # For TUI interfaces, empty response indicates cancel
     if [ $exit_status -ne 0 ] || [[ -z "$CHOSEN_ITEM" ]]; then
@@ -222,7 +232,7 @@ function radiolist() {
       shift
       shift
     done
-    CHOSEN_ITEM=$( zenity --title "$GUI_TITLE" $ZENITY_ICON_ARG "$GUI_ICON" --height="${ZENITY_HEIGHT-512}" ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --list --text "$TEXT" --radiolist --column "" --column "Value" --column "Description" "${OPTIONS[@]}" 2>/dev/null)
+    CHOSEN_ITEM=$( zenity --title "$GUI_TITLE" "$ZENITY_ICON_ARG" "$GUI_ICON" --height="${ZENITY_HEIGHT-512}" ${ZENITY_WIDTH+--width=$ZENITY_WIDTH} --list --text "$TEXT" --radiolist --column "" --column "Value" --column "Description" "${OPTIONS[@]}" 2>/dev/null)
     exit_status=$?
     # For GUI interfaces, empty response indicates cancel
     if [ $exit_status -ne 0 ] || [[ -z "$CHOSEN_ITEM" ]]; then