From b019b16a95b489f116cff50096d2f5157b999123 Mon Sep 17 00:00:00 2001
From: Grisha Levit <grishalevit@gmail.com>
Date: Fri, 25 Aug 2023 22:32:00 -0400
Subject: [PATCH] consistent BASH_COMMAND, DEBUG trap, xtrace order

There is some inconsistency betweeen various command types in the order of
(1) setting the value used for BASH_COMMAND (2) running the DEBUG trap and
checking its return value and (3) printing the PS4 string.

While simple, arithmetic, and conditional commands follow the order above,
`for', `select' and `case' do not.

For example, when executing a `for' command, BASH_COMMAND is only updated
after PS4 has been printed:

PS4=$'+ $BASH_COMMAND\t'
set -x
for ((i=0; i<1; i++)) { : i=$i; }

+ set -x        (( i=0 ))
+ ((i=0))       (( i<1 ))
+ : i=$i        : i=0
+ : i=$i        (( i++ ))
+ ((i++))       (( i<1 ))
---
 execute_cmd.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/execute_cmd.c b/execute_cmd.c
index ae5d313c..21b295f8 100644
--- a/execute_cmd.c
+++ b/execute_cmd.c
@@ -2954,9 +2954,6 @@ execute_for_command (FOR_COM *for_command)
       command_string_index = 0;
       print_for_command_head (for_command);
 
-      if (echo_command_at_execute)
-	xtrace_print_for_command_head (for_command);
-
       /* Save this command unless it's a trap command and we're not running
 	 a debug trap. */
       if (signal_in_progress (DEBUG_TRAP) == 0 && running_trap == 0)
@@ -2973,6 +2970,9 @@ execute_for_command (FOR_COM *for_command)
         continue;
 #endif
 
+      if (echo_command_at_execute)
+	xtrace_print_for_command_head (for_command);
+
       this_command_name = (char *)NULL;
       /* XXX - special ksh93 for command index variable handling */
       v = find_variable_last_nameref (identifier, 1);
@@ -3088,9 +3088,6 @@ eval_arith_for_expr (WORD_LIST *l, int *okp)
 
   if (new)
     {
-      if (echo_command_at_execute)
-	xtrace_print_arith_cmd (new);
-
       command_string_index = 0;
       print_arith_command (new);
       if (signal_in_progress (DEBUG_TRAP) == 0 && running_trap == 0)
@@ -3100,23 +3097,24 @@ eval_arith_for_expr (WORD_LIST *l, int *okp)
 	}
 
       r = run_debug_trap ();
+#if defined (DEBUGGER)
       /* In debugging mode, if the DEBUG trap returns a non-zero status, we
 	 skip the command. */
-      eflag = (shell_compatibility_level > 51) ? 0 : EXP_EXPANDED;
-      this_command_name = "((";		/* )) for expression error messages */
-      
-#if defined (DEBUGGER)
-      if (debugging_mode == 0 || r == EXECUTION_SUCCESS)
-	expresult = evalexp (new->word->word, eflag, okp);
-      else
+      if (debugging_mode && r != EXECUTION_SUCCESS)
 	{
 	  expresult = 0;
 	  if (okp)
 	    *okp = 1;
+	  return (expresult);
 	}
-#else
-      expresult = evalexp (new->word->word, eflag, okp);
 #endif
+      if (echo_command_at_execute)
+	xtrace_print_arith_cmd (new);
+
+      eflag = (shell_compatibility_level > 51) ? 0 : EXP_EXPANDED;
+      this_command_name = "((";		/* )) for expression error messages */
+
+      expresult = evalexp (new->word->word, eflag, okp);
       dispose_words (new);
     }
   else
@@ -3425,9 +3423,6 @@ execute_select_command (SELECT_COM *select_command)
   command_string_index = 0;
   print_select_command_head (select_command);
 
-  if (echo_command_at_execute)
-    xtrace_print_select_command_head (select_command);
-
 #if 0
   if (signal_in_progress (DEBUG_TRAP) == 0 && (this_command_name == 0 || (STREQ (this_command_name, "trap") == 0)))
 #else
@@ -3446,6 +3441,9 @@ execute_select_command (SELECT_COM *select_command)
     return (EXECUTION_SUCCESS);
 #endif
 
+  if (echo_command_at_execute)
+    xtrace_print_select_command_head (select_command);
+
   this_command_name = (char *)0;
 
   loop_level++; interrupt_execution++;
@@ -3565,9 +3563,6 @@ execute_case_command (CASE_COM *case_command)
   command_string_index = 0;
   print_case_command_head (case_command);
 
-  if (echo_command_at_execute)
-    xtrace_print_case_command_head (case_command);
-
 #if 0
   if (signal_in_progress (DEBUG_TRAP) == 0 && (this_command_name == 0 || (STREQ (this_command_name, "trap") == 0)))
 #else
@@ -3589,6 +3584,9 @@ execute_case_command (CASE_COM *case_command)
     }
 #endif
 
+  if (echo_command_at_execute)
+    xtrace_print_case_command_head (case_command);
+
   /* Use the same expansions (the ones POSIX specifies) as the patterns;
      dequote the resulting string (as POSIX specifies) since the quotes in
      patterns are handled specially below. We have to do it in this order
-- 
2.42.0