mirror of
https://github.com/fish-shell/fish-shell
synced 2025-01-15 22:44:01 +00:00
highlighter: underline valid "cd" arguments also if they come from CDPATH
When visiting the "cd" node, we mark invalid paths as error, but don't underline valid paths. This works fine most of the time because we later underline paths (for any command, not just "cd"). However the latter check fails to honor CDPATH. Let's correct that, which also allows to simplify the logic.
This commit is contained in:
parent
dfb0c00d72
commit
861ac00a61
2 changed files with 24 additions and 23 deletions
|
@ -5348,6 +5348,7 @@ static void test_highlighting() {
|
|||
if (!pushd("test/fish_highlight_test/")) return;
|
||||
cleanup_t pop{[] { popd(); }};
|
||||
if (system("mkdir -p dir")) err(L"mkdir failed");
|
||||
if (system("mkdir -p cdpath-entry/dir-in-cdpath")) err(L"mkdir failed");
|
||||
if (system("touch foo")) err(L"touch failed");
|
||||
if (system("touch bar")) err(L"touch failed");
|
||||
|
||||
|
@ -5418,6 +5419,11 @@ static void test_highlighting() {
|
|||
{L"definitely_not_a_directory", highlight_role_t::error},
|
||||
});
|
||||
|
||||
highlight_tests.push_back({
|
||||
{L"cd", highlight_role_t::command},
|
||||
{L"dir-in-cdpath", param_valid_path},
|
||||
});
|
||||
|
||||
// Command substitutions.
|
||||
highlight_tests.push_back({
|
||||
{L"echo", highlight_role_t::command},
|
||||
|
@ -5691,6 +5697,7 @@ static void test_highlighting() {
|
|||
// Verify variables and wildcards in commands using /bin/cat.
|
||||
vars.set(L"VARIABLE_IN_COMMAND", ENV_LOCAL, {L"a"});
|
||||
vars.set(L"VARIABLE_IN_COMMAND2", ENV_LOCAL, {L"at"});
|
||||
vars.set(L"CDPATH", ENV_LOCAL, {L"./cdpath-entry"});
|
||||
highlight_tests.push_back(
|
||||
{{L"/bin/ca", highlight_role_t::command, ns}, {L"*", highlight_role_t::operat, ns}});
|
||||
|
||||
|
|
|
@ -1008,18 +1008,31 @@ void highlighter_t::visit(const ast::semi_nl_t &semi_nl) {
|
|||
|
||||
void highlighter_t::visit(const ast::argument_t &arg, bool cmd_is_cd, bool options_allowed) {
|
||||
color_as_argument(arg, options_allowed);
|
||||
if (cmd_is_cd && io_still_ok()) {
|
||||
if (!io_still_ok()) {
|
||||
return;
|
||||
}
|
||||
// Underline every valid path.
|
||||
bool is_valid_path = false;
|
||||
if (cmd_is_cd) {
|
||||
// Mark this as an error if it's not 'help' and not a valid cd path.
|
||||
wcstring param = arg.source(this->buff);
|
||||
if (expand_one(param, expand_flag::skip_cmdsubst, ctx)) {
|
||||
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, ctx, PATH_EXPAND_TILDE)) {
|
||||
if (!is_help) {
|
||||
is_valid_path =
|
||||
is_potential_cd_path(param, working_directory, ctx, PATH_EXPAND_TILDE);
|
||||
if (!is_valid_path) {
|
||||
this->color_node(arg, highlight_role_t::error);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if (range_is_potential_path(buff, arg.range, ctx, working_directory)) {
|
||||
is_valid_path = true;
|
||||
}
|
||||
if (is_valid_path)
|
||||
for (size_t i = arg.range.start, end = arg.range.start + arg.range.length; i < end; i++)
|
||||
this->color_array.at(i).valid_path = true;
|
||||
}
|
||||
|
||||
void highlighter_t::visit(const ast::variable_assignment_t &varas) {
|
||||
|
@ -1269,25 +1282,6 @@ highlighter_t::color_array_t highlighter_t::highlight() {
|
|||
this->color_range(r, highlight_role_t::error);
|
||||
}
|
||||
|
||||
// Underline every valid path.
|
||||
if (io_still_ok()) {
|
||||
for (const ast::node_t &node : ast) {
|
||||
const auto arg = node.try_as<ast::argument_t>();
|
||||
if (!arg || arg->unsourced) continue;
|
||||
if (ctx.check_cancel()) break;
|
||||
if (range_is_potential_path(buff, arg->range, ctx, working_directory)) {
|
||||
// Don't color highlight_role_t::error because it looks dorky. For example,
|
||||
// trying to cd into a non-directory would show an underline and also red.
|
||||
for (size_t i = arg->range.start, end = arg->range.start + arg->range.length;
|
||||
i < end; i++) {
|
||||
if (this->color_array.at(i).foreground != highlight_role_t::error) {
|
||||
this->color_array.at(i).valid_path = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return std::move(color_array);
|
||||
}
|
||||
} // namespace
|
||||
|
|
Loading…
Reference in a new issue