From 7fea321b3e58f5801bf7544d7e73cedbed5221df Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 21 Mar 2021 09:25:29 +0100 Subject: [PATCH] Use the correct case in completion pager (#7744) Consider $ complete -c foo -a 'aab aaB' -f $ foo A since 28d67c8 we would insert the common prefix AND show the pager. Due to case-insensitive comparison, "b/B" was considered to be part of the prefix. Since the prefix is added to each pager item [1] we get wrong results. Fix this by removing the insensitive comparison between completions - I don't think it was of much use anyway. Commandline tokens are still matched case-insensitively, this is just about completions. Test this by running interactive fish inside tmux (pexpect's terminal emulation not have enough capabilities). Also add tests for recent interactive regressions #7526 and #7738. Closes #3978 [1]: b38a23a would solve this differently by giving every pager item its own prefix, but was reverted since it needs more fixes. --- CHANGELOG.rst | 1 + src/reader.cpp | 10 +------ tests/checks/tmux-complete.fish | 49 +++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 tests/checks/tmux-complete.fish diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8cf1832a3..f98d39a24 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,7 @@ Scripting improvements Interactive improvements ------------------------- +- When there are multiple completion candidates, fish inserts their shared prefix. This prefix was computed in a case-insensitive way, resulting in wrong case in the completion pager. This was fixed by only inserting prefixes with matching case (:issue:`7744`). New or improved bindings ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/reader.cpp b/src/reader.cpp index b1167c2bc..ecce6814d 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1996,15 +1996,7 @@ bool reader_data_t::handle_completions(const completion_list_t &comp, size_t tok 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; + if (common_prefix.at(idx) != el.completion.at(idx)) break; } // idx is now the length of the new common prefix. diff --git a/tests/checks/tmux-complete.fish b/tests/checks/tmux-complete.fish new file mode 100644 index 000000000..b9c38d680 --- /dev/null +++ b/tests/checks/tmux-complete.fish @@ -0,0 +1,49 @@ +#RUN: %fish -C 'set -g fish %fish' %s +#REQUIRES: command -v tmux + +# Isolated tmux. +set -g tmpdir (mktemp -d) +set -g tmux tmux -S $tmpdir/.tmux-socket -f /dev/null + +set -g sleep sleep .3 # TSan tests in the CI failed with .1. + +set fish (realpath $fish) +cd $tmpdir + +$tmux new-session -d $fish -C ' + # This is similar to "tests/interactive.config". + function fish_greeting; end + function fish_prompt; printf "prompt $status_generation> "; end + # No autosuggestion from older history. + set fish_history "" +' +$tmux resize-window -x 80 -y 10 +$sleep # Let fish draw a prompt. + +# Don't escape existing token (#7526). +echo >file-1 +echo >file-2 +$tmux send-keys 'HOME=$PWD ls ~/' Tab +$sleep +$tmux capture-pane -p +# CHECK: prompt 0> HOME=$PWD ls ~/file- +# CHECK: ~/file-1 ~/file-2 + +# No pager on single smartcase completion (#7738). +$tmux send-keys C-u C-l 'mkdir cmake CMakeFiles' Enter C-l \ + 'cat cmake' Tab +$sleep +$tmux capture-pane -p +# CHECK: prompt 1> cat cmake/ + +# Correct case in pager when prefies differ in case (#7743). +$tmux send-keys C-u C-l 'complete -c foo2 -a "aabc aaBd" -f' Enter C-l \ + 'foo2 A' Tab +$sleep +$tmux capture-pane -p +# The "bc" part is the autosuggestion - we could use "tmux capture-pane -e" to check colors. +# CHECK: prompt 2> foo2 aabc +# CHECK: aabc aaBd + +$tmux kill-server +rm -r $tmpdir