Improve pav cd command

lib/pavilion/commands/cd.sh

@@ -1,16 +1,62 @@
-#!/usr/bin/env bash
+# Test that the user is using bash
+[[ -n $BASH_VERSION ]] || {
+    printf "pav cd requires bash." >&2
+    return 1
+}
 
+# Disable color output when output isn't a terminal
+if [[ -t 2 ]]; then
+    RED=$'\033[0;31m'
+    RESET=$'\033[0m'
+else
+    RED='' RESET=''
+fi
+
+
+# pav: Wrapper around pav CLI to enable `cd` command
+#
+# Usage:
+#   pav cd [TEST_ID]
+#   pav <args...>
+#
+# Description:
+#   If called as `pav cd`, resolves the test run directory and cds into it.
+#   Otherwise, forwards all arguments to the underlying pav command.
+#
+# Arguments:
+#   TEST_ID   Optional test identifier. If not provided, defaults to the most recent test.
+#
+# Returns:
+#   0 on success, non-zero on failure.
 pav() {
-    # If "--help" flag is provided to cd, fall through to cd-help command
-    if [[ $1 = "cd" && $# -le 2 && $2 != "--help" ]]; then
-        test_path=$($PAVBIN/pav ls --path $2)
+    local test_path
+    local cmd=${1-}
+    local test_id=${2-}
 
-        if [[ $? -eq 0 ]]; then
-            cd $test_path
+    # If "--help" flag is provided to cd, fall through to cd-help command
+    if [[ $cmd == cd && $# -le 2 && $test_id != --help ]]; then
+        if [[ -n $test_id ]]; then
+            if test_path="$("$PAVBIN/pav" ls --path "$test_id")"; then
+                builtin cd -- "$test_path" || {
+                    printf '%sUnable to cd to test run directory for test: %s%s.\n' "$RED" "$test_id" "$RESET" >&2
+                    return 1
+                }
+            else
+                printf '%sUnable to cd to test run directory for test: %s%s.\n' "$RED" "$test_id" "$RESET" >&2
+                return 1
+            fi
         else
-            echo -e "\e[0;31mUnable to cd to test run directory for test \`$2\`.\e[0m"
+            if test_path="$("$PAVBIN/pav" ls --path)"; then
+                builtin cd -- "$test_path" || {
+                    printf '%sUnable to cd to directory for most recent test.%s\n' "$RED" "$RESET" >&2
+                    return 1
+                }
+            else
+                printf '%sUnable to cd to directory for most recent test.%s\n' "$RED" "$RESET" >&2
+                return 1
+            fi
         fi
     else
-        $PAVBIN/pav $@
+        "$PAVBIN/pav" "$@"
     fi
 }

lib/pavilion/commands/cd_help.py

@@ -7,24 +7,34 @@
 
 
 class CDHelpCommand(Command):
-    """This command exists solely to provide help information for the cd command, which due to
-    technical reasons must be implemented in bash."""
+    """Display help and usage information for the `cd` command, which must be activated from the
+    `cd.sh` script."""
 
     def __init__(self):
         super().__init__(
             "cd",
-            "Change to the test directory of the test with the given ID.",
-            short_help="Change to test directory")
+            "Change to the test run directory of the test with the given ID.",
+            short_help="Change to test run directory")
 
     def _setup_arguments(self, parser: ArgumentParser) -> None:
         """Set up the arguments for the cd command."""
 
-        parser.add_argument("test_id", type=TestID, help="test ID", nargs="?")
+        parser.add_argument("test_id", type=TestID,
+                            help="Test ID of the test run directory to change to. "
+                                 "If no ID is given, defaults to the most recent test.",
+                            nargs="?")
 
-    def run(self, pav_cfg: PavConfig, args: Namespace) -> None:
-        """Dummy method to run the cd command. This should never be run. Instead, pav cd
-        invokes a bash function."""
+    def run(self, pav_cfg: PavConfig, args: Namespace) -> int:
+        """Dummy method to display an error message if `pav cd` has not been activated.
+        During normal use, this method will not be called, and `cd.sh` will be invoked instead."""
 
         output.fprint(self.errfile,
-                      "You must source the activate script before running the cd command.",
+                      "The pav cd command must be activated before use. To activate, add the "
+                      "following line to your activate.sh script, then source the script:\n\n"
+                      "\tsource \"${PAV_CONFIG_DIR}/pav_src/lib/pavilion/commands/cd.sh\"\n\n"
+                      "If you do not have an activate.sh script, you can generate one by running "
+                      "the following command:\n\n"
+                      "\tpav make-activate",
                       color=output.RED)
+
+        return 1