Skip to content

Commit

Permalink
Hacky but functional tab completion that works with mixed cases
Browse files Browse the repository at this point in the history 
Fixes fish-shell#7944
  • Loading branch information
@razzius
razzius committed Aug 9, 2023
1 parent f9d21cc commit ae27633
Showing 1 changed file with 5 additions and 54 deletions.
59 changes: 5 additions & 54 deletions src/reader.cpp
View file
Expand Up @@ -2323,16 +2323,6 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok

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.
completion_list_t surviving_completions;
Expand All @@ -2341,12 +2331,8 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok
// 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;

// Don't use completions that want to replace, if we cannot replace them.
if (completion_replace_token && !reader_can_replace(tok, el.flags)) continue;
if (!reader_can_replace(tok, el.flags)) continue;

// This completion survived.
surviving_completions.push_back(el);
Expand All @@ -2370,18 +2356,14 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok
return true;
}

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.
Expand All @@ -2393,52 +2375,21 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok

// 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();
}
}

if (use_prefix) {
for (completion_t &c : surviving_completions) {
c.flags &= ~COMPLETE_REPLACES_TOKEN;
c.completion.erase(0, common_prefix.size());
for (completion_t &c : surviving_completions) {
if (!(c.flags & COMPLETE_REPLACES_TOKEN)) {
c.completion = tok + c.completion;
c.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);
}

// Update the pager data.
pager.set_prefix(prefix);
pager.set_completions(surviving_completions);
// Modify the command line to reflect the new pager.
pager_selection_changed();
Expand Down

2 comments on commit ae27633

@krackers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@razzius You can try my fork https://github.com/krackers/fish-shell/tree/3.3.1, see the last 3 commits; it solves this in a way that still preserves prefix insertion by borrowing ridiculousfish's idea of uniformly treating everything as COMPLETE_REPLACES_TOKEN by concat'ing the current token the non-replace ones.

Tbh I feel things could be even simpler, if we are just willing to force all completions to use COMPLETE_REPLACES_TOKEN. From what I can see, the only thing that will be affected is completion for shell variables and globs. E.g. instead of cd $HOME/[^] returning completions as $HOME/foo, $HOME/bar if you force the COMPLETE_REPLACES_TOKEN codepath then all completions will have $HOME expanded. I.e. it will behave more like an abbreviation. Personally I don't think this behavior is bad, but some people may not like it.

@razzius
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @krackers, thanks for commenting and thanks for your in the comment on the issue which I based this off of :)

I'd like to avoid expanding tokens in the completion; sometimes I want to have specific unexpanded variables in my history.

The issue with my approach and the reason I haven't tried to upstream this yet is the loss of the underlined prefix as I comment on here: fish-shell#7944 (comment)

Regardless the code can definitely be simplified, it's a hack to set the COMPLETE_REPLACES_TOKEN flag even for completions that don't replace the token. But I'm a bit over my head here in c++... and it doesn't help that the only test for this, tests/checks/tmux-complete.fish is said to be flaky and is disabled in CI, and I get this error I don't understand when I run it:

  5/234 Test #181: tmux-complete.fish........................***Failed    3.41 sec
Testing file checks/tmux-complete.fish ... Failure:

  The CHECK on line 16 wants:
    prompt 0> HOME=$PWD ls ~/file-{{1?}}

  which failed to match line stdout:1:
    prompt 0> HOME=$PWD ls ~/temp/

  Context:
    prompt 0> HOME=$PWD ls ~/temp/ <= does not match CHECK on line 16: prompt 0> HOME=$PWD ls ~/file-{{1?}}
    ~/file-1  ~/file-2
    prompt 1> cat cmake/
    cmake/  CMakeFiles/ <= does not match CHECK on line 32: prompt 2> foo2 aabc
    prompt 2> foo2 Aabc <= no check matches this, previous check on line 32
    aabc  aaBd
    prompt 3> begin
    rows 1 to 6 of 10
    prompt 3> foo2 <= does not match CHECK on line 53: prompt 3> foo2 aa
    prompt 3> foo2 aabc aabc
    aabc  aaBd
              foo4 b-short-arg a-long-arg-________________________________________…

  when running command:
    ../test/root/bin/fish checks/tmux-complete.fish

Please sign in to comment.