From 4f718e83b343cd2cf49c801968dd36cbce84a772 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Mon, 7 Oct 2013 03:56:09 -0700
Subject: [PATCH] Syntax highlighting now correctly handles cd
---
fish_tests.cpp | 27 +++++++++++++++-
highlight.cpp | 83 ++++++++++++++++++++++++++++++++++++--------------
parse_tree.cpp | 14 +++++++++
parse_tree.h | 3 ++
4 files changed, 103 insertions(+), 24 deletions(-)
diff --git a/fish_tests.cpp b/fish_tests.cpp
index 894408591..bc631bf32 100644
--- a/fish_tests.cpp
+++ b/fish_tests.cpp
@@ -2003,7 +2003,32 @@ static void test_highlighting(void)
{NULL, -1}
};
- const highlight_component_t *tests[] = {components1, components2, components3};
+ /* Verify that cd shows errors for non-directories */
+ const highlight_component_t components4[] =
+ {
+ {L"cd", HIGHLIGHT_COMMAND},
+ {L"/tmp/fish_highlight_test", HIGHLIGHT_PARAM | HIGHLIGHT_VALID_PATH},
+ {NULL, -1}
+ };
+
+ const highlight_component_t components5[] =
+ {
+ {L"cd", HIGHLIGHT_COMMAND},
+ {L"/tmp/fish_highlight_test/foo", HIGHLIGHT_ERROR},
+ {NULL, -1}
+ };
+
+ const highlight_component_t components6[] =
+ {
+ {L"cd", HIGHLIGHT_COMMAND},
+ {L"--help", HIGHLIGHT_PARAM},
+ {L"-h", HIGHLIGHT_PARAM},
+ {L"definitely_not_a_directory", HIGHLIGHT_ERROR},
+ {NULL, -1}
+ };
+
+
+ const highlight_component_t *tests[] = {components1, components2, components3, components4, components5, components6};
for (size_t which = 0; which < sizeof tests / sizeof *tests; which++)
{
const highlight_component_t *components = tests[which];
diff --git a/highlight.cpp b/highlight.cpp
index a8e8326e1..71dba3dcf 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -1467,7 +1467,7 @@ static void color_node(const parse_node_t &node, int color, std::vector<int> &co
std::fill(color_array.begin() + node.source_start, color_array.begin() + source_end, color);
}
-/* This function is a disaster badly in need of refactoring */
+/* This function is a disaster badly in need of refactoring. However, note that it does NOT do any I/O */
static void color_argument(const wcstring &buffstr, std::vector<int>::iterator colors, int normal_status)
{
const size_t buff_len = buffstr.size();
@@ -1772,10 +1772,45 @@ static bool node_is_potential_path(const wcstring &src, const parse_node_t &node
return result;
}
-// Color all of the arguments of the given command
-static void color_arguments(const wcstring &src, const parse_node_tree_t &tree, const parse_node_t &parent, std::vector<int> &color_array)
+// Gets the expanded command from a plain statement node
+static bool plain_statement_get_expanded_command(const wcstring &src, const parse_node_tree_t &tree, const parse_node_t &plain_statement, wcstring *out_cmd)
{
- const parse_node_tree_t::parse_node_list_t nodes = tree.find_nodes(parent, symbol_argument);
+ assert(plain_statement.type == symbol_plain_statement);
+ bool result = false;
+
+ // Get the command
+ const parse_node_t *cmd_node = tree.get_child(plain_statement, 0, parse_token_type_string);
+ if (cmd_node != NULL && cmd_node->has_source())
+ {
+ wcstring cmd(src, cmd_node->source_start, cmd_node->source_length);
+
+ /* Try expanding it. If we cannot, it's an error. */
+ if (expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS))
+ {
+ /* Success, return the expanded string by reference */
+ std::swap(cmd, *out_cmd);
+ result = true;
+ }
+ }
+ return result;
+}
+
+// Color all of the arguments of the given command
+static void color_arguments(const wcstring &src, const parse_node_tree_t &tree, const parse_node_t &list_node, const wcstring &working_directory, std::vector<int> &color_array)
+{
+ /* Hack: determine whether the parent is the cd command. */
+ bool cmd_is_cd = false;
+ const parse_node_t *parent = tree.get_parent(list_node, symbol_plain_statement);
+ if (parent != NULL)
+ {
+ wcstring cmd_str;
+ if (plain_statement_get_expanded_command(src, tree, *parent, &cmd_str))
+ {
+ cmd_is_cd = (cmd_str == L"cd");
+ }
+ }
+
+ const parse_node_tree_t::parse_node_list_t nodes = tree.find_nodes(list_node, symbol_argument);
wcstring param;
for (node_offset_t i=0; i < nodes.size(); i++)
@@ -1784,6 +1819,19 @@ static void color_arguments(const wcstring &src, const parse_node_tree_t &tree,
assert(child != NULL && child->type == symbol_argument);
param.assign(src, child->source_start, child->source_length);
color_argument(param, color_array.begin() + child->source_start, HIGHLIGHT_PARAM);
+
+ if (cmd_is_cd)
+ {
+ /* Mark this as an error if it's not 'help' and not a valid cd path */
+ if (expand_one(param, EXPAND_SKIP_CMDSUBST))
+ {
+ bool is_help = string_prefixes_string(param, L"--help") || string_prefixes_string(param, L"-h");
+ if (!is_help && ! is_potential_cd_path(param, working_directory, PATH_EXPAND_TILDE, NULL))
+ {
+ color_node(*child, HIGHLIGHT_ERROR, color_array);
+ }
+ }
+ }
}
}
@@ -1893,20 +1941,10 @@ void highlight_shell_magic(const wcstring &buff, std::vector<int> &color, size_t
case symbol_switch_statement:
case symbol_boolean_statement:
case symbol_decorated_statement:
- color_children(parse_tree, node, parse_token_type_string, HIGHLIGHT_COMMAND, color);
- break;
-
case symbol_if_statement:
{
// Color the 'end'
color_children(parse_tree, node, parse_token_type_string, HIGHLIGHT_COMMAND, color);
-
- // Color arguments and redirections
- const parse_node_t *arguments = parse_tree.get_child(node, 3, symbol_arguments_or_redirections_list);
- if (arguments != NULL)
- {
- color_arguments(buff, parse_tree, *arguments, color);
- }
}
break;
@@ -1948,21 +1986,20 @@ void highlight_shell_magic(const wcstring &buff, std::vector<int> &color, size_t
}
color_node(*cmd_node, is_valid_cmd ? HIGHLIGHT_COMMAND : HIGHLIGHT_ERROR, color);
}
-
- /* Color arguments */
- const parse_node_t *arguments = parse_tree.get_child(node, 1, symbol_arguments_or_redirections_list);
- if (arguments != NULL)
- {
- color_arguments(buff, parse_tree, *arguments, color);
- }
}
break;
case symbol_arguments_or_redirections_list:
case symbol_argument_list:
- /* Nothing, these are handled by their parents */
- break;
+ {
+ /* Only work on root lists, so that we don't re-color child lists */
+ if (parse_tree.argument_list_is_root(node))
+ {
+ color_arguments(buff, parse_tree, node, working_directory, color);
+ }
+ }
+ break;
case parse_special_type_parse_error:
case parse_special_type_tokenizer_error:
diff --git a/parse_tree.cpp b/parse_tree.cpp
index 0a85a1d95..5baef1c01 100644
--- a/parse_tree.cpp
+++ b/parse_tree.cpp
@@ -936,3 +936,17 @@ parse_node_tree_t::parse_node_list_t parse_node_tree_t::find_nodes(const parse_n
find_nodes_recursive(*this, parent, type, &result);
return result;
}
+
+
+bool parse_node_tree_t::argument_list_is_root(const parse_node_t &node) const
+{
+ bool result = true;
+ assert(node.type == symbol_argument_list || node.type == symbol_arguments_or_redirections_list);
+ const parse_node_t *parent = this->get_parent(node);
+ if (parent != NULL)
+ {
+ /* We have a parent - check to make sure it's not another list! */
+ result = parent->type != symbol_arguments_or_redirections_list && parent->type != symbol_argument_list;
+ }
+ return result;
+}
diff --git a/parse_tree.h b/parse_tree.h
index 6fcbde0dc..0355117fc 100644
--- a/parse_tree.h
+++ b/parse_tree.h
@@ -217,6 +217,9 @@ public:
/* Find all the nodes of a given type underneath a given node */
typedef std::vector<const parse_node_t *> parse_node_list_t;
parse_node_list_t find_nodes(const parse_node_t &parent, parse_token_type_t type) const;
+
+ /* Indicate if the given argument_list or arguments_or_redirections_list is a root list, or has a parent */
+ bool argument_list_is_root(const parse_node_t &node) const;
};