github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2024-11-22 14:43:55 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Attempt to simplify how completions get presented in the pager

Browse Source 
This is an attempt to simplfy some completion logic. It mainly refactors
reader_data_t::handle_completions such that all completions have the token
prepended; this attempts to simplify the logic since now all completions
replace the token. It also changes how the pager prefix works. Previously
the pager prefix was an extra string that was prepended to all
completions. In the new model the completions already have the prefix
prepended and the prefix is used only for certain width calculations.

This is a somewhat frightening change in an interactive component with
low test coverage. It tweaks things like how long completions are
ellipsized. Buckle in!
This commit is contained in:
ridiculousfish 2020-11-28 20:12:27 -08:00
parent 4b947e0a23
commit b38a23a46d
5 changed files with 128 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/fish_tests.cpp
View File

@ -2289,7 +2289,7 @@ static void test_pager_navigation() {
}
pager_t pager;
pager.set_completions(completions);
pager.set_completions(completions, wcstring{});
pager.set_term_size(termsize_t::defaults());
page_rendering_t render = pager.render();
@ -2410,7 +2410,7 @@ static void test_pager_layout() {
// These test cases have equal completions and descriptions
const completion_t c1(L"abcdefghij", L"1234567890");
pager.set_completions(completion_list_t(1, c1));
pager.set_completions(completion_list_t{c1}, wcstring{});
const pager_layout_testcase_t testcases1[] = {
{26, L"abcdefghij (1234567890)"}, {25, L"abcdefghij (1234567890)"},
{24, L"abcdefghij (1234567890)"}, {23, L"abcdefghij (12345678…)"},
@ -2425,7 +2425,7 @@ static void test_pager_layout() {
// These test cases have heavyweight completions
const completion_t c2(L"abcdefghijklmnopqrs", L"1");
pager.set_completions(completion_list_t(1, c2));
pager.set_completions(completion_list_t{c2}, wcstring{});
const pager_layout_testcase_t testcases2[] = {
{26, L"abcdefghijklmnopqrs (1)"}, {25, L"abcdefghijklmnopqrs (1)"},
{24, L"abcdefghijklmnopqrs (1)"}, {23, L"abcdefghijklmnopq… (1)"},
@ -2440,7 +2440,7 @@ static void test_pager_layout() {
// These test cases have no descriptions
const completion_t c3(L"abcdefghijklmnopqrst", L"");
pager.set_completions(completion_list_t(1, c3));
pager.set_completions(completion_list_t{c3}, wcstring{});
const pager_layout_testcase_t testcases3[] = {
{26, L"abcdefghijklmnopqrst"}, {25, L"abcdefghijklmnopqrst"},
{24, L"abcdefghijklmnopqrst"}, {23, L"abcdefghijklmnopqrst"},

74
src/pager.cpp
View File

@ -39,6 +39,10 @@ using comp_info_list_t = std::vector<comp_t>;
/// Text we use for the search field.
#define SEARCH_FIELD_PROMPT _(L"search: ")
/// Maximum length of prefix string when printing completion list. Longer prefixes will be
/// ellipsized.
#define PREFIX_MAX_LEN 9
inline bool selection_direction_is_cardinal(selection_motion_t dir) {
switch (dir) {
case selection_motion_t::north:
@ -75,8 +79,10 @@ static size_t divide_round_up(size_t numer, size_t denom) {
/// \param max the maximum space that may be used for printing
/// \param has_more if this flag is true, this is not the entire string, and the string should be
/// ellipsized even if the string fits but takes up the whole space.
/// \param prefix_len Hack: if nonzero, then color the first prefix_len chars with the prefix color.
/// \param prefix_color the color to use for the first prefix_len characters.
static size_t print_max(const wcstring &str, highlight_spec_t color, size_t max, bool has_more,
line_t *line) {
line_t *line, size_t prefix_len = 0, highlight_spec_t prefix_color = {}) {
size_t remaining = max;
for (size_t i = 0; i < str.size(); i++) {
wchar_t c = str.at(i);
@ -97,7 +103,7 @@ static size_t print_max(const wcstring &str, highlight_spec_t color, size_t max,
break;
}
line->append(c, color);
line->append(c, i < prefix_len ? prefix_color : color);
assert(remaining >= width_c);
remaining -= width_c;
}
@ -108,8 +114,8 @@ static size_t print_max(const wcstring &str, highlight_spec_t color, size_t max,
}
/// Print the specified item using at the specified amount of space.
line_t pager_t::completion_print_item(const wcstring &prefix, const comp_t *c, size_t row,
size_t column, size_t width, bool secondary, bool selected,
line_t pager_t::completion_print_item(const comp_t *c, size_t row, size_t column, size_t width,
bool secondary, bool selected,
page_rendering_t *rendering) const {
UNUSED(column);
UNUSED(row);
@ -157,19 +163,16 @@ line_t pager_t::completion_print_item(const wcstring &prefix, const comp_t *c, s
highlight_spec_t comp_col = {modify_role(highlight_role_t::pager_completion), bg_role};
highlight_spec_t desc_col = {modify_role(highlight_role_t::pager_description), bg_role};
// Print the completion part
// Print the completion part.
size_t comp_remaining = comp_width;
for (size_t i = 0; i < c->comp.size(); i++) {
const wcstring &comp = c->comp.at(i);
if (i > 0) {
comp_remaining -=
print_max(PAGER_SPACER_STRING, bg, comp_remaining, true /* has_more */, &line_data);
}
comp_remaining -= print_max(prefix, prefix_col, comp_remaining, !comp.empty(), &line_data);
comp_remaining -=
print_max(comp, comp_col, comp_remaining, i + 1 < c->comp.size(), &line_data);
comp_remaining -= print_max(comp, comp_col, comp_remaining, i + 1 < c->comp.size(),
&line_data, c->prefix_len, prefix_col);
}
size_t desc_remaining = width - comp_width + comp_remaining;
@ -203,10 +206,9 @@ line_t pager_t::completion_print_item(const wcstring &prefix, const comp_t *c, s
/// \param width_by_column An array specifying the width of each column
/// \param row_start The first row to print
/// \param row_stop the row after the last row to print
/// \param prefix The string to print before each completion
/// \param lst The list of completions to print
void pager_t::completion_print(size_t cols, const size_t *width_by_column, size_t row_start,
size_t row_stop, const wcstring &prefix, const comp_info_list_t &lst,
size_t row_stop, const comp_info_list_t &lst,
page_rendering_t *rendering) const {
// Teach the rendering about the rows it printed.
assert(row_stop >= row_start);
@ -226,7 +228,7 @@ void pager_t::completion_print(size_t cols, const size_t *width_by_column, size_
bool is_selected = (idx == effective_selected_idx);
// Print this completion on its own "line".
line_t line = completion_print_item(prefix, el, row, col, width_by_column[col], row % 2,
line_t line = completion_print_item(el, row, col, width_by_column[col], row % 2,
is_selected, rendering);
// If there's more to come, append two spaces.
@ -300,7 +302,8 @@ static void join_completions(comp_info_list_t *comps) {
}
/// Generate a list of comp_t structures from a list of completions.
static comp_info_list_t process_completions_into_infos(const completion_list_t &lst) {
static comp_info_list_t process_completions_into_infos(const completion_list_t &lst,
size_t prefix_len) {
const size_t lst_size = lst.size();
// Make the list of the correct size up-front.
@ -308,9 +311,22 @@ static comp_info_list_t process_completions_into_infos(const completion_list_t &
for (size_t i = 0; i < lst_size; i++) {
const completion_t &comp = lst.at(i);
comp_t *comp_info = &result.at(i);
comp_info->prefix_len = prefix_len;
// Perhaps ellipsize the prefix.
// FIXME: The escaping mucks with the length here; we may color the wrong number of
// characters. Prefix should be based on width not length anyways.
wcstring comp_str = escape_string(comp.completion, ESCAPE_NO_QUOTED);
if (prefix_len > PREFIX_MAX_LEN && comp_str.size() > PREFIX_MAX_LEN) {
// Discard the prefix, except for the last PREFIX_MAX_LEN.
// Then ellipsize the first char.
comp_str.erase(0, prefix_len - PREFIX_MAX_LEN);
comp_str.at(0) = get_ellipsis_char();
comp_info->prefix_len = PREFIX_MAX_LEN;
}
// Append the single completion string. We may later merge these into multiple.
comp_info->comp.push_back(escape_string(comp.completion, ESCAPE_NO_QUOTED));
comp_info->comp.push_back(std::move(comp_str));
// Append the mangled description.
comp_info->desc = comp.description;
@ -322,8 +338,7 @@ static comp_info_list_t process_completions_into_infos(const completion_list_t &
return result;
}
void pager_t::measure_completion_infos(comp_info_list_t *infos, const wcstring &prefix) const {
size_t prefix_len = fish_wcswidth(prefix);
void pager_t::measure_completion_infos(comp_info_list_t *infos) const {
for (auto &info : *infos) {
comp_t *comp = &info;
const wcstring_list_t &comp_strings = comp->comp;
@ -334,7 +349,7 @@ void pager_t::measure_completion_infos(comp_info_list_t *infos, const wcstring &
// fish_wcswidth() can return -1 if it can't calculate the width. So be cautious.
int comp_width = fish_wcswidth(comp_strings.at(j));
if (comp_width >= 0) comp->comp_width += prefix_len + comp_width;
if (comp_width >= 0) comp->comp_width += comp_width;
}
// fish_wcswidth() can return -1 if it can't calculate the width. So be cautious.
@ -357,7 +372,7 @@ bool pager_t::completion_info_passes_filter(const comp_t &info) const {
// Match against the completion strings.
for (const auto &i : info.comp) {
if (string_fuzzy_match_string(needle, prefix + i)) {
if (string_fuzzy_match_string(needle, i)) {
return true;
}
}
@ -375,31 +390,31 @@ void pager_t::refilter_completions() {
}
}
void pager_t::set_completions(const completion_list_t &raw_completions) {
void pager_t::set_completions(const completion_list_t &raw_completions,
const wcstring &shared_prefix) {
// Get completion infos out of it.
unfiltered_completion_infos = process_completions_into_infos(raw_completions);
unfiltered_completion_infos =
process_completions_into_infos(raw_completions, shared_prefix.size());
// Maybe join them.
if (prefix == L"-") join_completions(&unfiltered_completion_infos);
if (shared_prefix == L"-") join_completions(&unfiltered_completion_infos);
// Compute their various widths.
measure_completion_infos(&unfiltered_completion_infos, prefix);
measure_completion_infos(&unfiltered_completion_infos);
// Refilter them.
this->refilter_completions();
}
void pager_t::set_prefix(const wcstring &pref) { prefix = pref; }
void pager_t::set_term_size(termsize_t ts) {
available_term_width = ts.width > 0 ? ts.width : 0;
available_term_height = ts.height > 0 ? ts.height : 0;
}
/// Try to print the list of completions lst with the prefix prefix using cols as the number of
/// Try to print the list of completions \p lst using \p cols as the number of
/// columns. Return true if the completion list was printed, false if the terminal is too narrow for
/// the specified number of columns. Always succeeds if cols is 1.
bool pager_t::completion_try_print(size_t cols, const wcstring &prefix, const comp_info_list_t &lst,
bool pager_t::completion_try_print(size_t cols, const comp_info_list_t &lst,
page_rendering_t *rendering, size_t suggested_start_row) const {
assert(cols > 0);
// The calculated preferred width of each column.
@ -480,7 +495,7 @@ bool pager_t::completion_try_print(size_t cols, const wcstring &prefix, const co
assert(stop_row >= start_row);
assert(stop_row <= row_count);
assert(stop_row - start_row <= term_height);
completion_print(cols, width_by_column, start_row, stop_row, prefix, lst, rendering);
completion_print(cols, width_by_column, start_row, stop_row, lst, rendering);
// Add the progress line. It's a "more to disclose" line if necessary, or a row listing if
// it's scrollable; otherwise ignore it.
@ -566,7 +581,7 @@ page_rendering_t pager_t::render() const {
rendering.selected_completion_idx =
this->visual_selected_completion_index(rendering.rows, rendering.cols);
if (completion_try_print(cols, prefix, completion_infos, &rendering, suggested_row_start)) {
if (completion_try_print(cols, completion_infos, &rendering, suggested_row_start)) {
break;
}
}
@ -836,7 +851,6 @@ size_t pager_t::get_selected_column(const page_rendering_t &rendering) const {
void pager_t::clear() {
unfiltered_completion_infos.clear();
completion_infos.clear();
prefix.clear();
selected_completion_idx = PAGER_SELECTION_NONE;
fully_disclosed = false;
search_field_shown = false;

25
src/pager.h
View File

@ -91,6 +91,9 @@ class pager_t {
size_t comp_width{0};
/// On-screen width of the description information.
size_t desc_width{0};
/// Length of the shared prefix for each completion.
/// These characters are colored using pager_prefix highlight role.
size_t prefix_len{0};
// Our text looks like this:
// completion (description)
@ -116,32 +119,26 @@ class pager_t {
// The unfiltered list. Note there's a lot of duplication here.
comp_info_list_t unfiltered_completion_infos;
wcstring prefix;
bool completion_try_print(size_t cols, const wcstring &prefix, const comp_info_list_t &lst,
page_rendering_t *rendering, size_t suggested_start_row) const;
bool completion_try_print(size_t cols, const comp_info_list_t &lst, page_rendering_t *rendering,
size_t suggested_start_row) const;
void recalc_min_widths(comp_info_list_t *lst) const;
void measure_completion_infos(std::vector<comp_t> *infos, const wcstring &prefix) const;
void measure_completion_infos(std::vector<comp_t> *infos) const;
bool completion_info_passes_filter(const comp_t &info) const;
void completion_print(size_t cols, const size_t *width_by_column, size_t row_start,
size_t row_stop, const wcstring &prefix, const comp_info_list_t &lst,
size_t row_stop, const comp_info_list_t &lst,
page_rendering_t *rendering) const;
line_t completion_print_item(const wcstring &prefix, const comp_t *c, size_t row, size_t column,
size_t width, bool secondary, bool selected,
page_rendering_t *rendering) const;
line_t completion_print_item(const comp_t *c, size_t row, size_t column, size_t width,
bool secondary, bool selected, page_rendering_t *rendering) const;
public:
// The text of the search field.
editable_line_t search_field_line;
// Sets the set of completions.
void set_completions(const completion_list_t &raw_completions);
// Sets the prefix.
void set_prefix(const wcstring &pref);
// Sets the set of completions, and their shared prefix.
void set_completions(const completion_list_t &raw_completions, const wcstring &prefix);
// Sets the terminal size.
void set_term_size(termsize_t ts);

164
src/reader.cpp
View File

@ -82,10 +82,6 @@
// interactive command to complete.
#define ENV_CMD_DURATION L"CMD_DURATION"
/// Maximum length of prefix string when printing completion list. Longer prefixes will be
/// ellipsized.
#define PREFIX_MAX_LEN 9
/// A simple prompt for reading shell commands that does not rely on fish specific commands, meaning
/// it will work even if fish is not installed. This is used by read_i.
#define DEFAULT_PROMPT L"echo -n \"$USER@$hostname $PWD \"'> '"
@ -1814,6 +1810,45 @@ static uint32_t get_best_rank(const completion_list_t &comp) {
return best_rank;
}
/// \return the common string prefix of a list of completions.
static wcstring extract_common_prefix(const completion_list_t &completions) {
bool has_seed = false;
wcstring result;
// Seed it with the first samecase completion (if any), so that the prefix has the same case as
// the command line.
for (const completion_t &c : completions) {
if (c.match.is_samecase()) {
result = c.completion;
has_seed = true;
break;
}
}
for (const completion_t &c : completions) {
if (!has_seed) {
result = c.completion;
has_seed = true;
continue;
}
// Allow case insensitive common prefix if our completion was not samecase.
bool icase = !c.match.is_samecase();
size_t i = 0;
size_t max = std::min(c.completion.size(), result.size());
for (; i < max; i++) {
wchar_t c1 = c.completion[i];
wchar_t c2 = result[i];
bool chars_match = (c1 == c2 || (icase && towlower(c1) == towlower(c2)));
if (!chars_match) {
break;
}
}
assert(i <= result.size() && "Shared prefix should not make string longer");
result.resize(i);
}
return result;
}
/// Handle the list of completions. This means the following:
///
/// - If the list is empty, flash the terminal.
@ -1835,7 +1870,7 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok
bool success = false;
const editable_line_t *el = &command_line;
const wcstring tok(el->text().c_str() + token_begin, token_end - token_begin);
const wcstring tok(el->text(), token_begin, token_end - token_begin);
// Check trivial cases.
size_t size = comp.size();
@ -1860,118 +1895,51 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok
return success;
}
auto best_rank = get_best_rank(comp);
// Determine whether we are going to replace the token or not. If any commands of the best
// rank do not require replacement, then ignore all those that want to use replacement.
bool will_replace_token = true;
for (const completion_t &el : comp) {
if (el.rank() <= best_rank && !(el.flags & COMPLETE_REPLACES_TOKEN)) {
will_replace_token = false;
break;
}
}
// Decide which completions survived. There may be a lot of them; it would be nice if we could
// figure out how to avoid copying them here.
auto best_rank = get_best_rank(comp);
completion_list_t surviving_completions;
bool all_matches_exact_or_prefix = true;
for (const completion_t &el : comp) {
for (const completion_t &c : comp) {
// Ignore completions with a less suitable match rank than the best.
if (el.rank() > best_rank) continue;
// Only use completions that match replace_token.
bool completion_replace_token = static_cast<bool>(el.flags & COMPLETE_REPLACES_TOKEN);
if (completion_replace_token != will_replace_token) continue;
if (c.rank() > best_rank) continue;
// Don't use completions that want to replace, if we cannot replace them.
if (completion_replace_token && !reader_can_replace(tok, el.flags)) continue;
bool completion_replace_token = (c.flags & COMPLETE_REPLACES_TOKEN);
if (completion_replace_token && !reader_can_replace(tok, c.flags)) continue;
// This completion survived.
surviving_completions.push_back(el);
all_matches_exact_or_prefix = all_matches_exact_or_prefix && el.match.is_exact_or_prefix();
surviving_completions.push_back(c);
all_matches_exact_or_prefix = all_matches_exact_or_prefix && c.match.is_exact_or_prefix();
}
bool use_prefix = false;
wcstring common_prefix;
if (all_matches_exact_or_prefix) {
// Try to find a common prefix to insert among the surviving completions.
complete_flags_t flags = 0;
bool prefix_is_partial_completion = false;
bool first = true;
for (const completion_t &el : surviving_completions) {
if (first) {
// First entry, use the whole string.
common_prefix = el.completion;
flags = el.flags;
first = false;
} else {
// Determine the shared prefix length.
size_t idx, max = std::min(common_prefix.size(), el.completion.size());
for (idx = 0; idx < max; idx++) {
wchar_t ac = common_prefix.at(idx), bc = el.completion.at(idx);
bool matches = (ac == bc);
// If we are replacing the token, allow case to vary.
if (will_replace_token && !matches) {
// Hackish way to compare two strings in a case insensitive way,
// hopefully better than towlower().
matches = (wcsncasecmp(&ac, &bc, 1) == 0);
}
if (!matches) break;
}
// idx is now the length of the new common prefix.
common_prefix.resize(idx);
prefix_is_partial_completion = true;
// Early out if we decide there's no common prefix.
if (idx == 0) break;
}
}
// Determine if we use the prefix. We use it if it's non-empty and it will actually make
// the command line longer. It may make the command line longer by virtue of not using
// REPLACE_TOKEN (so it always appends to the command line), or by virtue of replacing
// the token but being longer than it.
use_prefix = common_prefix.size() > (will_replace_token ? tok.size() : 0);
assert(!use_prefix || !common_prefix.empty());
if (use_prefix) {
// We got something. If more than one completion contributed, then it means we have
// a prefix; don't insert a space after it.
if (prefix_is_partial_completion) flags |= COMPLETE_NO_SPACE;
completion_insert(common_prefix, token_end, flags);
cycle_command_line = command_line.text();
cycle_cursor_pos = command_line.position();
// Ensure that all surviving completions replace their token, so we can handle them uniformly.
for (completion_t &c : surviving_completions) {
if (!(c.flags & COMPLETE_REPLACES_TOKEN)) {
c.flags |= COMPLETE_REPLACES_TOKEN;
c.completion.insert(0, tok);
}
}
if (use_prefix) {
for (completion_t &c : surviving_completions) {
c.flags &= ~COMPLETE_REPLACES_TOKEN;
c.completion.erase(0, common_prefix.size());
}
}
// Compute the common prefix (perhaps empty) of all surviving completions, and replace our token
// with it if it would make the token longer.
wcstring common_prefix = extract_common_prefix(surviving_completions);
if (common_prefix.size() > tok.size()) {
complete_flags_t flags = COMPLETE_REPLACES_TOKEN;
// Print the completion list.
wcstring prefix;
if (will_replace_token || !all_matches_exact_or_prefix) {
if (use_prefix) prefix = std::move(common_prefix);
} else if (tok.size() + common_prefix.size() <= PREFIX_MAX_LEN) {
prefix = tok + common_prefix;
} else {
// Append just the end of the string.
prefix = wcstring{get_ellipsis_char()};
prefix.append(tok + common_prefix, tok.size() + common_prefix.size() - PREFIX_MAX_LEN,
PREFIX_MAX_LEN);
// Replace the token! Note this invalidates token_begin and token_end.
// Do not insert a space if more than one completion contributed.
if (surviving_completions.size() > 1) flags |= COMPLETE_NO_SPACE;
completion_insert(common_prefix, token_end, flags);
cycle_command_line = command_line.text();
cycle_cursor_pos = command_line.position();
}
// Update the pager data.
pager.set_prefix(prefix);
pager.set_completions(surviving_completions);
pager.set_completions(surviving_completions, common_prefix);
// Invalidate our rendering.
current_page_rendering = page_rendering_t();
current_page_rendering = page_rendering_t{};
// Modify the command line to reflect the new pager.
pager_selection_changed();
return false;

3
src/wcstringutil.h
View File

@ -67,6 +67,9 @@ struct string_fuzzy_match_t {
return type == contain_type_t::exact && case_fold == case_fold_t::samecase;
}
/// \return if this is a samecase completion.
bool is_samecase() const { return case_fold == case_fold_t::samecase; }
/// \return if we are exact or prefix match.
bool is_exact_or_prefix() const {
switch (type) {