Highlight options differently

This introduces a new variable, $fish_color_option, that can be used
to highlight options differently.

Options are tokens starting with `-`, but only up to (and including!)
the first `--`.

Fixes #8292.
This commit is contained in:
Fabian Homborg 2021-10-19 17:20:21 +02:00
parent 1888bda3e6
commit 711796ad13
6 changed files with 41 additions and 11 deletions

View file

@ -37,6 +37,7 @@ ROLE_TO_TOKEN = {
"keyword": Keyword, "keyword": Keyword,
"statement_terminator": Punctuation, "statement_terminator": Punctuation,
"param": Name.Constant, "param": Name.Constant,
"option": Name.Literal,
"comment": Comment, "comment": Comment,
"match": DEFAULT, "match": DEFAULT,
"search_match": DEFAULT, "search_match": DEFAULT,

View file

@ -107,6 +107,7 @@ Variable Meaning
``fish_color_end`` process separators like ``;`` and ``&`` ``fish_color_end`` process separators like ``;`` and ``&``
``fish_color_error`` syntax errors ``fish_color_error`` syntax errors
``fish_color_param`` ordinary command parameters ``fish_color_param`` ordinary command parameters
``fish_color_option`` options starting with "-", up to the first "--" parameter
``fish_color_comment`` comments like '# important' ``fish_color_comment`` comments like '# important'
``fish_color_selection`` selected text in vi visual mode ``fish_color_selection`` selected text in vi visual mode
``fish_color_operator`` parameter expansion operators like ``*`` and ``~`` ``fish_color_operator`` parameter expansion operators like ``*`` and ``~``
@ -120,7 +121,10 @@ Variable Meaning
``fish_color_search_match`` history search matches and selected pager items (background only) ``fish_color_search_match`` history search matches and selected pager items (background only)
========================================== ===================================================================== ========================================== =====================================================================
If a variable isn't set, fish usually tries ``$fish_color_normal``, except for ``$fish_color_keyword``, where it tries ``$fish_color_command`` first. If a variable isn't set, fish usually tries ``$fish_color_normal``, except for:
- ``$fish_color_keyword``, where it tries ``$fish_color_command`` first.
- ``$fish_color_option``, where it tries ``$fish_color_param`` first.
.. _variables-color-pager: .. _variables-color-pager:

View file

@ -671,6 +671,7 @@ static const char *highlight_role_to_string(highlight_role_t role) {
TEST_ROLE(keyword) TEST_ROLE(keyword)
TEST_ROLE(statement_terminator) TEST_ROLE(statement_terminator)
TEST_ROLE(param) TEST_ROLE(param)
TEST_ROLE(option)
TEST_ROLE(comment) TEST_ROLE(comment)
TEST_ROLE(search_match) TEST_ROLE(search_match)
TEST_ROLE(operat) TEST_ROLE(operat)
@ -777,6 +778,9 @@ static const wchar_t *html_class_name_for_color(highlight_spec_t spec) {
case highlight_role_t::param: { case highlight_role_t::param: {
return P(param); return P(param);
} }
case highlight_role_t::option: {
return P(option);
}
case highlight_role_t::comment: { case highlight_role_t::comment: {
return P(comment); return P(comment);
} }

View file

@ -5244,8 +5244,8 @@ static void test_highlighting() {
highlight_tests.push_back({ highlight_tests.push_back({
{L"cd", highlight_role_t::command}, {L"cd", highlight_role_t::command},
{L"--help", highlight_role_t::param}, {L"--help", highlight_role_t::option},
{L"-h", highlight_role_t::param}, {L"-h", highlight_role_t::option},
{L"definitely_not_a_directory", highlight_role_t::error}, {L"definitely_not_a_directory", highlight_role_t::error},
}); });
@ -5479,6 +5479,13 @@ static void test_highlighting() {
{L"# comment", highlight_role_t::comment}, {L"# comment", highlight_role_t::comment},
}); });
highlight_tests.push_back({
{L"echo", highlight_role_t::command},
{L"--", highlight_role_t::option},
{L"-s", highlight_role_t::param},
});
// Overlong paths don't crash (#7837). // Overlong paths don't crash (#7837).
const wcstring overlong = get_overlong_path(); const wcstring overlong = get_overlong_path();
highlight_tests.push_back({ highlight_tests.push_back({

View file

@ -51,6 +51,8 @@ static const wchar_t *get_highlight_var_name(highlight_role_t role) {
return L"fish_color_end"; return L"fish_color_end";
case highlight_role_t::param: case highlight_role_t::param:
return L"fish_color_param"; return L"fish_color_param";
case highlight_role_t::option:
return L"fish_color_option";
case highlight_role_t::comment: case highlight_role_t::comment:
return L"fish_color_comment"; return L"fish_color_comment";
case highlight_role_t::search_match: case highlight_role_t::search_match:
@ -111,6 +113,8 @@ static highlight_role_t get_fallback(highlight_role_t role) {
return highlight_role_t::command; return highlight_role_t::command;
case highlight_role_t::statement_terminator: case highlight_role_t::statement_terminator:
return highlight_role_t::normal; return highlight_role_t::normal;
case highlight_role_t::option:
return highlight_role_t::param;
case highlight_role_t::param: case highlight_role_t::param:
return highlight_role_t::normal; return highlight_role_t::normal;
case highlight_role_t::comment: case highlight_role_t::comment:
@ -539,7 +543,9 @@ static size_t color_variable(const wchar_t *in, size_t in_len,
static void color_string_internal(const wcstring &buffstr, highlight_spec_t base_color, static void color_string_internal(const wcstring &buffstr, highlight_spec_t base_color,
std::vector<highlight_spec_t>::iterator colors) { std::vector<highlight_spec_t>::iterator colors) {
// Clarify what we expect. // Clarify what we expect.
assert((base_color == highlight_role_t::param || base_color == highlight_role_t::command) && assert((base_color == highlight_role_t::param
|| base_color == highlight_role_t::option
|| base_color == highlight_role_t::command) &&
"Unexpected base color"); "Unexpected base color");
const size_t buff_len = buffstr.size(); const size_t buff_len = buffstr.size();
std::fill(colors, colors + buff_len, base_color); std::fill(colors, colors + buff_len, base_color);
@ -799,7 +805,7 @@ class highlighter_t {
// Color a command. // Color a command.
void color_command(const ast::string_t &node); void color_command(const ast::string_t &node);
// Color a node as if it were an argument. // Color a node as if it were an argument.
void color_as_argument(const ast::node_t &node); void color_as_argument(const ast::node_t &node, bool options_allowed = true);
// Colors the source range of a node with a given color. // Colors the source range of a node with a given color.
void color_node(const ast::node_t &node, highlight_spec_t color); void color_node(const ast::node_t &node, highlight_spec_t color);
// Colors a range with a given color. // Colors a range with a given color.
@ -824,7 +830,7 @@ class highlighter_t {
void visit(const ast::block_statement_t &block); void visit(const ast::block_statement_t &block);
// Visit an argument, perhaps knowing that our command is cd. // Visit an argument, perhaps knowing that our command is cd.
void visit(const ast::argument_t &arg, bool cmd_is_cd = false); void visit(const ast::argument_t &arg, bool cmd_is_cd = false, bool options_allowed = true);
// Default implementation is to just visit children. // Default implementation is to just visit children.
void visit(const ast::node_t &node) { visit_children(node); } void visit(const ast::node_t &node) { visit_children(node); }
@ -867,7 +873,7 @@ void highlighter_t::color_command(const ast::string_t &node) {
} }
// node does not necessarily have type symbol_argument here. // node does not necessarily have type symbol_argument here.
void highlighter_t::color_as_argument(const ast::node_t &node) { void highlighter_t::color_as_argument(const ast::node_t &node, bool options_allowed) {
auto source_range = node.source_range(); auto source_range = node.source_range();
const wcstring arg_str = get_source(source_range); const wcstring arg_str = get_source(source_range);
@ -876,7 +882,11 @@ void highlighter_t::color_as_argument(const ast::node_t &node) {
const color_array_t::iterator arg_colors = color_array.begin() + arg_start; const color_array_t::iterator arg_colors = color_array.begin() + arg_start;
// Color this argument without concern for command substitutions. // Color this argument without concern for command substitutions.
color_string_internal(arg_str, highlight_role_t::param, arg_colors); if (options_allowed && arg_str[0] == L'-') {
color_string_internal(arg_str, highlight_role_t::option, arg_colors);
} else {
color_string_internal(arg_str, highlight_role_t::param, arg_colors);
}
// Now do command substitutions. // Now do command substitutions.
size_t cmdsub_cursor = 0, cmdsub_start = 0, cmdsub_end = 0; size_t cmdsub_cursor = 0, cmdsub_start = 0, cmdsub_end = 0;
@ -1001,8 +1011,8 @@ void highlighter_t::visit(const ast::semi_nl_t &semi_nl) {
color_node(semi_nl, highlight_role_t::statement_terminator); color_node(semi_nl, highlight_role_t::statement_terminator);
} }
void highlighter_t::visit(const ast::argument_t &arg, bool cmd_is_cd) { void highlighter_t::visit(const ast::argument_t &arg, bool cmd_is_cd, bool options_allowed) {
color_as_argument(arg); color_as_argument(arg, options_allowed);
if (cmd_is_cd && io_ok) { if (cmd_is_cd && io_ok) {
// Mark this as an error if it's not 'help' and not a valid cd path. // Mark this as an error if it's not 'help' and not a valid cd path.
wcstring param = arg.source(this->buff); wcstring param = arg.source(this->buff);
@ -1065,6 +1075,8 @@ void highlighter_t::visit(const ast::decorated_statement_t &stmt) {
// Except if our command is 'cd' we have special logic for how arguments are colored. // Except if our command is 'cd' we have special logic for how arguments are colored.
bool is_cd = (expanded_cmd == L"cd"); bool is_cd = (expanded_cmd == L"cd");
bool is_set = (expanded_cmd == L"set"); bool is_set = (expanded_cmd == L"set");
// If we have seen a "--" argument, color all options from then on as normal arguments.
bool have_dashdash = false;
for (const ast::argument_or_redirection_t &v : stmt.args_or_redirs) { for (const ast::argument_or_redirection_t &v : stmt.args_or_redirs) {
if (v.is_argument()) { if (v.is_argument()) {
if (is_set) { if (is_set) {
@ -1074,7 +1086,8 @@ void highlighter_t::visit(const ast::decorated_statement_t &stmt) {
is_set = false; is_set = false;
} }
} }
this->visit(v.argument(), is_cd); this->visit(v.argument(), is_cd, !have_dashdash);
if (v.argument().source(this->buff) == L"--") have_dashdash = true;
} else { } else {
this->visit(v.redirection()); this->visit(v.redirection());
} }

View file

@ -20,6 +20,7 @@ enum class highlight_role_t : uint8_t {
keyword, keyword,
statement_terminator, // process separator statement_terminator, // process separator
param, // command parameter (argument) param, // command parameter (argument)
option, // argument starting with "-", up to a "--"
comment, // comment comment, // comment
search_match, // search match search_match, // search match
operat, // operator operat, // operator