Expand the variable name as an ordinary parameter in for loops. Added

test for it too.

5 files changed, 23 insertions, 14 deletions

diff --git a/highlight.cpp b/highlight.cpp
index e8afa203..eaaec750 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -1333,19 +1333,9 @@ const highlighter_t::color_array_t & highlighter_t::highlight()
                 this->color_node(*literal_for_node, highlight_spec_command);
                 this->color_node(*literal_in_node, highlight_spec_command);
                 
-                // Color the variable name appropriately
-                // We don't expand this during execution, so just look for invalid chars
+                // Color the variable name as a parameter
                 const parse_node_t *var_name_node = this->parse_tree.get_child(node, 1, parse_token_type_string);
-                if (var_name_node->has_source())
-                {
-                    size_t source_end = var_name_node->source_start + var_name_node->source_length;
-                    for (size_t i = var_name_node->source_start; i < source_end; i++)
-                    {
-                        wchar_t wc = buff.at(i);
-                        highlight_spec_t color = wcsvarchr(wc) ? highlight_spec_param : highlight_spec_error;
-                        color_array.at(i) = color;
-                    }
-                }
+                this->color_argument(*var_name_node);
             }
             break;
 
diff --git a/parse_constants.h b/parse_constants.h
index 32115c01..884b76f7 100644
--- a/parse_constants.h
+++ b/parse_constants.h
@@ -157,6 +157,9 @@ typedef unsigned int parser_test_error_bits_t;
 /** Error message when encountering an illegal command name */
 #define ILLEGAL_CMD_ERR_MSG _( L"Illegal command name '%ls'")
 
+/** Error message when encountering a failed expansion, e.g. for the variable name in for loops */
+#define FAILED_EXPANSION_VARIABLE_NAME_ERR_MSG _( L"Unable to expand variable name '%ls'")
+
 /** Error message when encountering an illegal file descriptor */
 #define ILLEGAL_FD_ERR_MSG _( L"Illegal file descriptor in redirection '%ls'")
 
diff --git a/parse_execution.cpp b/parse_execution.cpp
index f2eaae25..726f2920 100644
--- a/parse_execution.cpp
+++ b/parse_execution.cpp
@@ -436,9 +436,14 @@ parse_execution_result_t parse_execution_context_t::run_for_statement(const pars
     assert(header.type == symbol_for_header);
     assert(block_contents.type == symbol_job_list);
 
-    /* Get the variable name: `for var_name in ...` */
+    /* Get the variable name: `for var_name in ...`. We expand the variable name. It better result in just one. */
     const parse_node_t &var_name_node = *get_child(header, 1, parse_token_type_string);
-    const wcstring for_var_name = get_source(var_name_node);
+    wcstring for_var_name = get_source(var_name_node);
+    if (! expand_one(for_var_name, 0))
+    {
+        report_error(var_name_node, FAILED_EXPANSION_VARIABLE_NAME_ERR_MSG, for_var_name.c_str());
+        return parse_execution_errored;
+    }
 
     /* Get the contents to iterate over. */
     const parse_node_t *unmatched_wildcard = NULL;
diff --git a/tests/test9.in b/tests/test9.in
index e449a21d..32a537f7 100644
--- a/tests/test9.in
+++ b/tests/test9.in
@@ -70,4 +70,13 @@ end
 # Test implicit cd. This should do nothing.
 ./
 
+# Test special for loop expansion
+# Here we the name of the variable is derived from another variable
+echo "Testing for loop"
+set var1 var2
+for $var1 in 1 2 3
+    echo -n $var2
+end
+echo
+
 false
diff --git a/tests/test9.out b/tests/test9.out
index cf9054f8..e44b2604 100644
--- a/tests/test9.out
+++ b/tests/test9.out
@@ -8,3 +8,5 @@ Foop
 Foop
 Foop
 Doop
+Testing for loop
+123