Skip to content

Commit

Permalink
Use the correct case in completion pager
Browse files Browse the repository at this point in the history
Consider

	$ complete -c foo -a 'aab aaB' -f
	$ foo A<TAB>

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.
  • Loading branch information
krobelus committed Mar 21, 2021
1 parent 211f8bc commit a4d0fdd
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
10 changes: 1 addition & 9 deletions src/reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
49 changes: 49 additions & 0 deletions tests/checks/tmux-complete.fish
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a4d0fdd

Please sign in to comment.