konevsa/fish
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

highlight: underline prefixes of valid paths only if at cursor

Browse Source 
As the user is typing an argument, fish continually checks if the input is
the prefix of a valid file path. If yes, the input is underlined.

The same prefix-logic is used for all tokens on the command line, even for
"finished" tokens. This means we highlight any token that happens to be
a prefix of a valid file path. We actually want this to only apply to the
token that the user is currently typing.

Let's use the prefix-logic only for tokens adjacent to the cursor.  This should
better match user expectations (and reduce IO traffic). I don't think this is
the perfect criteria but I don't know how else we can determine if a token is
"unfinished".
This commit is contained in:
Johannes Altmanninger 2022-10-26 12:29:36 +02:00
parent 6667c9f50c
commit f637fb31b5
4 changed files with 54 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/fish_tests.cpp
View File

@ -2870,25 +2870,26 @@ static void test_is_potential_path() {
const wcstring_list_t wds({L".", wd});
operation_context_t ctx{env_stack_t::principal()};
do_test(is_potential_path(L"al", wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"alpha/", wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"aard", wds, ctx, 0));
do_test(is_potential_path(L"al", true, wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"alpha/", true, wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"aard", true, wds, ctx, 0));
do_test(!is_potential_path(L"aard", false, wds, ctx, 0));
do_test(!is_potential_path(L"balpha/", wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"aard", wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"aarde", wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"aarde", wds, ctx, 0));
do_test(!is_potential_path(L"balpha/", true, wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"aard", true, wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"aarde", true, wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"aarde", true, wds, ctx, 0));
do_test(is_potential_path(L"test/is_potential_path_test/aardvark", wds, ctx, 0));
do_test(is_potential_path(L"test/is_potential_path_test/al", wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"test/is_potential_path_test/aardv", wds, ctx, 0));
do_test(is_potential_path(L"test/is_potential_path_test/aardvark", true, wds, ctx, 0));
do_test(is_potential_path(L"test/is_potential_path_test/al", true, wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"test/is_potential_path_test/aardv", true, wds, ctx, 0));
do_test(
!is_potential_path(L"test/is_potential_path_test/aardvark", wds, ctx, PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"test/is_potential_path_test/al/", wds, ctx, 0));
do_test(!is_potential_path(L"test/is_potential_path_test/ar", wds, ctx, 0));
do_test(!is_potential_path(L"test/is_potential_path_test/aardvark", true, wds, ctx,
PATH_REQUIRE_DIR));
do_test(!is_potential_path(L"test/is_potential_path_test/al/", true, wds, ctx, 0));
do_test(!is_potential_path(L"test/is_potential_path_test/ar", true, wds, ctx, 0));
do_test(is_potential_path(L"/usr", wds, ctx, PATH_REQUIRE_DIR));
do_test(is_potential_path(L"/usr", true, wds, ctx, PATH_REQUIRE_DIR));
}
/// Test the 'test' builtin.
@ -5610,6 +5611,13 @@ static void test_highlighting() {
{L"2>", highlight_role_t::redirection},
});
// Highlight path-prefixes only at the cursor.
highlight_tests.push_back({
{L"cat", highlight_role_t::command},
{L"/dev/nu", highlight_role_t::param},
{L"/dev/nu", param_valid_path},
});
highlight_tests.push_back({
{L"if", highlight_role_t::keyword},
{L"true", highlight_role_t::command},

36
src/highlight.cpp
View File

@ -185,8 +185,9 @@ static bool fs_is_case_insensitive(const wcstring &path, int fd,
/// the deepest unique directory hierarchy.
///
/// We expect the path to already be unescaped.
bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_list_t &directories,
const operation_context_t &ctx, path_flags_t flags) {
bool is_potential_path(const wcstring &potential_path_fragment, bool at_cursor,
const wcstring_list_t &directories, const operation_context_t &ctx,
path_flags_t flags) {
ASSERT_IS_BACKGROUND_THREAD();
if (ctx.check_cancel()) return false;
@ -244,11 +245,16 @@ bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_l
if (abs_path.empty() || checked_paths.count(abs_path)) continue;
checked_paths.insert(abs_path);
// If we end with a slash, then it must be a directory.
// If the user is still typing the argument, we want to highlight it if it's the prefix
// of a valid path. This means we need to potentially walk all files in some directory.
// There are two easy cases where we can skip this:
// 1. If the argument ends with a slash, it must be a valid directory, no prefix.
// 2. If the cursor is not at the argument, it means the user is definitely not typing it,
// so we can skip the prefix-match.
bool must_be_full_dir = abs_path.at(abs_path.size() - 1) == L'/';
if (must_be_full_dir) {
if (must_be_full_dir || !at_cursor) {
struct stat buf;
if (0 == wstat(abs_path, &buf) && S_ISDIR(buf.st_mode)) {
if (0 == wstat(abs_path, &buf) && (!at_cursor || S_ISDIR(buf.st_mode))) {
return true;
}
} else {
@ -290,8 +296,9 @@ bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_l
// Given a string, return whether it prefixes a path that we could cd into. Return that path in
// out_path. Expects path to be unescaped.
static bool is_potential_cd_path(const wcstring &path, const wcstring &working_directory,
const operation_context_t &ctx, path_flags_t flags) {
static bool is_potential_cd_path(const wcstring &path, bool at_cursor,
const wcstring &working_directory, const operation_context_t &ctx,
path_flags_t flags) {
wcstring_list_t directories;
if (string_prefixes_string(L"./", path)) {
@ -313,7 +320,8 @@ static bool is_potential_cd_path(const wcstring &path, const wcstring &working_d
}
// Call is_potential_path with all of these directories.
return is_potential_path(path, directories, ctx, flags | PATH_REQUIRE_DIR | PATH_FOR_CD);
return is_potential_path(path, at_cursor, directories, ctx,
flags | PATH_REQUIRE_DIR | PATH_FOR_CD);
}
// Given a plain statement node in a parse tree, get the command and return it, expanded
@ -929,7 +937,7 @@ void highlighter_t::color_as_argument(const ast::node_t &node, bool options_allo
/// Indicates whether the source range of the given node forms a valid path in the given
/// working_directory.
static bool range_is_potential_path(const wcstring &src, const source_range_t &range,
const operation_context_t &ctx,
bool at_cursor, const operation_context_t &ctx,
const wcstring &working_directory) {
// Skip strings exceeding PATH_MAX. See #7837.
// Note some paths may exceed PATH_MAX, but this is just for highlighting.
@ -946,7 +954,8 @@ static bool range_is_potential_path(const wcstring &src, const source_range_t &r
if (!token.empty() && token.at(0) == HOME_DIRECTORY) token.at(0) = L'~';
const wcstring_list_t working_directory_list(1, working_directory);
result = is_potential_path(token, working_directory_list, ctx, PATH_EXPAND_TILDE);
result =
is_potential_path(token, at_cursor, working_directory_list, ctx, PATH_EXPAND_TILDE);
}
return result;
}
@ -1021,6 +1030,7 @@ void highlighter_t::visit(const ast::argument_t &arg, bool cmd_is_cd, bool optio
}
// Underline every valid path.
bool is_valid_path = false;
bool at_cursor = cursor.has_value() && arg.source_range().contains_inclusive(*cursor);
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);
@ -1028,14 +1038,14 @@ void highlighter_t::visit(const ast::argument_t &arg, bool cmd_is_cd, bool optio
bool is_help =
string_prefixes_string(param, L"--help") || string_prefixes_string(param, L"-h");
if (!is_help) {
is_valid_path =
is_potential_cd_path(param, working_directory, ctx, PATH_EXPAND_TILDE);
is_valid_path = is_potential_cd_path(param, at_cursor, 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)) {
} else if (range_is_potential_path(buff, arg.range, at_cursor, ctx, working_directory)) {
is_valid_path = true;
}
if (is_valid_path)

5
src/highlight.h
View File

@ -146,7 +146,8 @@ enum {
PATH_FOR_CD = 1 << 2,
};
typedef unsigned int path_flags_t;
bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_list_t &directories,
const operation_context_t &ctx, path_flags_t flags);
bool is_potential_path(const wcstring &potential_path_fragment, bool at_cursor,
const wcstring_list_t &directories, const operation_context_t &ctx,
path_flags_t flags);
#endif

5
src/parse_constants.h
View File

@ -31,6 +31,11 @@ struct source_range_t {
}
bool operator!=(const source_range_t &rhs) const { return !(*this == rhs); }
// \return true if a location is in this range, including one-past-the-end.
bool contains_inclusive(source_offset_t loc) const {
return start <= loc && loc - start <= length;
}
};
// IMPORTANT: If the following enum table is modified you must also update token_enum_map below.