Created
April 7, 2025 15:15
-
-
Save krobelus/16bb43409004d694faa73535b6a1c450 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
╭────────────────────────────────────────────╮ | |
│ ✻ Welcome to Claude Code research preview! │ | |
│ │ | |
│ /help for help │ | |
│ │ | |
│ cwd: /home/johannes/git/fish-shell │ | |
╰────────────────────────────────────────────╯ | |
> /code | |
● I'm Claude Code, ready to help with your software engineering tasks. Type /help for more information, or just tell me what you need help with. | |
> fix this issue: { | |
"number": 11343, | |
"title": "Pager/completion search does not properly consider `.`", | |
"createdAt": "2025-04-01T19:17:32Z", | |
"state": "OPEN", | |
"url": "https://github.com/fish-shell/fish-shell/issues/11343", | |
"author": { | |
"login": "mqudsi" | |
}, | |
"body": "With the presence of a literal match to the search string (second filtered pager entry visible below), I don't think the first of the two results should have been shown at all (unless we change from \"filter\" to \"search | |
and sort by priority\" and then it is displayed second):\n\n", | |
"comments": { | |
"nodes": [] | |
} | |
}, | |
✻ Thinking… | |
This issue is about the fish shell's pager/completion search functionality. The problem is that when searching for a string containing a . character, the search doesn't properly filter the results. Specifically, entries that don't | |
contain the exact searched string (with the .) are still showing up in the results, even when there are literal matches available. | |
From the issue description, it seems like when the user searches for a string with a ., items that don't contain the exact . character are still being shown in the results (the first entry), while there's an entry that has a literal | |
match (the second entry). The issue suggests that either: | |
1. The non-literal match shouldn't be shown at all, OR | |
2. If we want to show both, we should prioritize the literal match first. | |
To fix this issue, I need to: | |
1. Find the code that handles pager/completion search | |
2. Understand how it currently handles the . character (it's likely treating it as a special character rather than a literal) | |
3. Modify the code to either filter out non-literal matches or prioritize literal matches higher | |
Let's start by searching for pager and completion-related code in the fish-shell repository. | |
● I'll investigate and fix the issue with how the pager/completion search handles the . character. Let me find the relevant code. | |
● Search(pattern: "pager.*search", include: "*.rs")… | |
⎿ Found 9 files | |
● Let's check the pager.rs file as it's likely to contain the search functionality: | |
● Read(file_path: "src/pager.rs")… | |
⎿ //! Pager support. | |
use std::collections::hash_map::Entry; | |
... (+1261 lines) | |
● Now let's look at the string fuzzy match functionality since that's likely where the issue occurs: | |
● Read(file_path: "src/wcstringutil.rs")… | |
⎿ //! Helper functions for working with wcstring. | |
use crate::common::{get_ellipsis_char, get_ellipsis_str}; | |
... (+743 lines) | |
● Now let's look at how the string fuzzy matching is used in the pager: | |
● Read(file_path: "src/pager.rs", offset: 368, limit: 20)… | |
⎿ // Indicates if the given completion info passes any filtering we have. | |
fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
... (+17 lines) | |
● I've identified the issue! Let me explain the problem and implement a fix. | |
The issue is in the way the pager fuzzy matching works when a . character is present in the search string. Looking at the issue description, it appears that the pager is not properly prioritizing exact matches with . characters. | |
The problem is in the string_fuzzy_match_string function in wcstringutil.rs. When it performs the fuzzy match, it doesn't consider . as a special character that should be matched literally. The current implementation treats it as a | |
regular character. | |
Here's my fix: | |
● Update(file_path: src/wcstringutil.rs)… | |
⎿ User rejected update to src/wcstringutil.rs | |
91 if needle.is_empty() { | |
92 return Some(0); | |
93 } | |
94 // If needle contains a '.', first try an exact match (treat '.' literally) | |
95 if needle.contains('.') { | |
96 let exact_match = haystack | |
97 .as_char_slice() | |
98 .windows(needle.len()) | |
99 .position(|window| wstr::from_char_slice(window) == needle); | |
100 | |
101 if exact_match.is_some() { | |
102 return exact_match; | |
103 } | |
104 } | |
105 | |
106 haystack | |
107 .as_char_slice() | |
108 .windows(needle.len()) | |
> No, it's not about the . character being special. Rather, completion_info_passes_filter includes all completion candidates that are subsequence matches of the search string. This is unlike tab-completion, which ranks completion based on | |
whether they are exact substring or subsequence matches, and only shows candidates of the best non-empty category. The completion pager should have the same behavior. | |
● I understand the issue better now. The pager search doesn't properly prioritize matches like tab-completion does - it's showing all matches (subsequence, substring, etc.) without ranking them appropriately. Let me fix this by updating | |
the pager search to use the same ranking approach as tab-completion. | |
● Update(file_path: src/pager.rs)… | |
⎿ User rejected update to src/pager.rs | |
367 } | |
368 | |
369 // Indicates if the given completion info passes any filtering we have. | |
370 fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
371 // If we have no filter, everything passes. | |
370 // Returns a match quality rank (lower is better) or None if no match. | |
371 fn completion_info_match_quality(&self, info: &PagerComp) -> Option<u32> { | |
372 // If we have no filter, everything passes with best quality. | |
373 if !self.search_field_shown || self.search_field_line.is_empty() { | |
374 return true; | |
374 return Some(0); | |
375 } | |
376 | |
377 let needle = self.search_field_line.text(); | |
378 let mut best_rank = None; | |
379 | |
380 // Match against the description. | |
381 if string_fuzzy_match_string(needle, &info.desc, false).is_some() { | |
382 return true; | |
381 if let Some(match_info) = string_fuzzy_match_string(needle, &info.desc, false) { | |
382 best_rank = Some(match_info.rank()); | |
383 } | |
384 | |
385 // Match against the completion strings. | |
386 for candidate in &info.comp { | |
387 if string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) | |
388 .is_some() | |
389 { | |
390 return true; | |
387 if let Some(match_info) = string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) { | |
388 let rank = match_info.rank(); | |
389 best_rank = Some(match match_info.rank() { | |
390 r if best_rank.is_none() => r, | |
391 r if best_rank.unwrap() > r => r, // Lower rank is better | |
392 _ => best_rank.unwrap() | |
393 }); | |
394 } | |
395 } | |
396 false // no match | |
396 | |
397 best_rank | |
398 } | |
399 | |
400 // Indicates if the given completion info passes any filtering we have. | |
401 fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
402 // If we have no filter, everything passes. | |
403 if !self.search_field_shown || self.search_field_line.is_empty() { | |
404 return true; | |
405 } | |
406 | |
407 self.completion_info_match_quality(info).is_some() | |
408 } | |
409 } | |
410 | |
411 /// Print the specified part of the completion list, using the specified column offsets and quoting | |
412 /// style. | |
413 /// | |
> no, you need to first compute the best rank across all completions, pass that rank to completion_info_passes_filter, and have it return true only for completions of this given rank | |
● I understand now. We need to first find the best match quality across all completions, and then only include completions that match that best quality. Let me implement this approach: | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 69 additions and 1 removal | |
989 self.suggested_row_start = 0; | |
990 } | |
991 | |
992 // Gets match quality for a completion (lower is better, None means no match) | |
993 fn completion_info_match_quality(&self, info: &PagerComp) -> Option<u32> { | |
994 // If we have no filter, everything passes with best quality. | |
995 if !self.search_field_shown || self.search_field_line.is_empty() { | |
996 return Some(0); | |
997 } | |
998 | |
999 let needle = self.search_field_line.text(); | |
1000 let mut best_rank = None; | |
1001 | |
1002 // Match against the description. | |
1003 if let Some(match_info) = string_fuzzy_match_string(needle, &info.desc, false) { | |
1004 best_rank = Some(match_info.rank()); | |
1005 } | |
1006 | |
1007 // Match against the completion strings. | |
1008 for candidate in &info.comp { | |
1009 if let Some(match_info) = string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) { | |
1010 let rank = match_info.rank(); | |
1011 best_rank = match best_rank { | |
1012 None => Some(rank), | |
1013 Some(current_best) if rank < current_best => Some(rank), | |
1014 _ => best_rank, | |
1015 }; | |
1016 } | |
1017 } | |
1018 | |
1019 best_rank | |
1020 } | |
1021 | |
1022 // Checks if a completion passes the filter based on the best available match quality | |
1023 fn completion_info_passes_filter(&self, comp: &PagerComp, best_rank: u32) -> bool { | |
1024 if !self.search_field_shown || self.search_field_line.is_empty() { | |
1025 return true; | |
1026 } | |
1027 | |
1028 match self.completion_info_match_quality(comp) { | |
1029 Some(rank) if rank == best_rank => true, | |
1030 _ => false | |
1031 } | |
1032 } | |
1033 | |
1034 // Updates the completions list per the filter. | |
1035 pub fn refilter_completions(&mut self) { | |
1036 self.completion_infos.clear(); | |
1037 | |
1038 // If we have no filter, include everything | |
1039 if !self.search_field_shown || self.search_field_line.is_empty() { | |
1040 self.completion_infos = self.unfiltered_completion_infos.clone(); | |
1041 return; | |
1042 } | |
1043 | |
1044 // Find the best match quality across all completions | |
1045 let mut best_rank = None; | |
1046 for comp in &self.unfiltered_completion_infos { | |
1047 if self.completion_info_passes_filter(comp) { | |
1047 if let Some(rank) = self.completion_info_match_quality(comp) { | |
1048 best_rank = match best_rank { | |
1049 None => Some(rank), | |
1050 Some(current_best) if rank < current_best => Some(rank), | |
1051 _ => best_rank, | |
1052 }; | |
1053 } | |
1054 } | |
1055 | |
1056 // No matches found | |
1057 if best_rank.is_none() { | |
1058 return; | |
1059 } | |
1060 | |
1061 // Include only completions with the best match quality | |
1062 let best_rank = best_rank.unwrap(); | |
1063 for comp in &self.unfiltered_completion_infos { | |
1064 if self.completion_info_passes_filter(comp, best_rank) { | |
1065 self.completion_infos.push(comp.clone()); | |
1066 } | |
1067 } | |
● Now we have another issue: we still need to update the original completion_info_passes_filter function that is being used elsewhere. Let's update that: | |
● Search(pattern: "completion_info_passes_filter", include: "*.rs")… | |
⎿ Found 1 file | |
● Let's look for other usages of this function: | |
● Search(pattern: "\bcompletion_info_passes_filter\b", path: "src/pager.rs")… | |
⎿ Found 1 file | |
● Let's check the specific lines: | |
● Search(pattern: "self\.completion_info_passes_filter", path: "src/pager.rs")… | |
⎿ Found 1 file | |
● Let's update the signature of the original function to maintain backward compatibility: | |
● Update(file_path: src/pager.rs)… | |
⎿ User rejected update to src/pager.rs | |
1020 } | |
1021 | |
1022 // Checks if a completion passes the filter based on the best available match quality | |
1023 fn completion_info_passes_filter(&self, comp: &PagerComp, best_rank: u32) -> bool { | |
1023 fn completion_info_passes_filter_with_rank(&self, comp: &PagerComp, best_rank: u32) -> bool { | |
1024 if !self.search_field_shown || self.search_field_line.is_empty() { | |
1025 return true; | |
1026 } | |
... | |
1030 _ => false | |
1031 } | |
1032 } | |
1033 | |
1034 // The original function is retained for compatibility, but isn't used by refilter_completions | |
1035 fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
1036 // If we have no filter, everything passes. | |
1037 if !self.search_field_shown || self.search_field_line.is_empty() { | |
1038 return true; | |
1039 } | |
1040 | |
1041 self.completion_info_match_quality(info).is_some() | |
1042 } | |
1043 | |
1044 // Updates the completions list per the filter. | |
1045 pub fn refilter_completions(&mut self) { | |
1046 self.completion_infos.clear(); | |
> nah, just delete the old function, it's unused | |
● You're right, let's simplify the code by removing the old function. Let me fix the references in the existing code: | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 3 additions and 2 removals | |
1061 // Include only completions with the best match quality | |
1062 let best_rank = best_rank.unwrap(); | |
1063 for comp in &self.unfiltered_completion_infos { | |
1064 if self.completion_info_passes_filter(comp, best_rank) { | |
1065 self.completion_infos.push(comp.clone()); | |
1064 match self.completion_info_match_quality(comp) { | |
1065 Some(rank) if rank == best_rank => self.completion_infos.push(comp.clone()), | |
1066 _ => {} | |
1067 } | |
1068 } | |
1069 } | |
● Now let's remove the unused completion_info_passes_filter function: | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 11 removals | |
1019 best_rank | |
1020 } | |
1021 | |
1022 // Checks if a completion passes the filter based on the best available match quality | |
1023 fn completion_info_passes_filter(&self, comp: &PagerComp, best_rank: u32) -> bool { | |
1024 if !self.search_field_shown || self.search_field_line.is_empty() { | |
1025 return true; | |
1026 } | |
1027 | |
1028 match self.completion_info_match_quality(comp) { | |
1029 Some(rank) if rank == best_rank => true, | |
1030 _ => false | |
1031 } | |
1032 } | |
1022 | |
1023 // Updates the completions list per the filter. | |
1024 pub fn refilter_completions(&mut self) { | |
● Let's clean up the extra empty lines: | |
● Update(file_path: src/pager.rs)… | |
⎿ String to replace not found in file. | |
● Let me try again with the correct formatting: | |
● Update(file_path: src/pager.rs)… | |
⎿ String to replace not found in file. | |
● Let's check the current state of the file around that section: | |
● Read(file_path: "src/pager.rs", offset: 1010, limit: 20)… | |
⎿ let rank = match_info.rank(); | |
best_rank = match best_rank { | |
None => Some(rank), | |
... (+17 lines) | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 1 removal | |
1019 best_rank | |
1020 } | |
1021 | |
1022 | |
1022 // Updates the completions list per the filter. | |
1023 pub fn refilter_completions(&mut self) { | |
1024 self.completion_infos.clear(); | |
● Let's build the project to make sure our changes compile: | |
● Bash(cd /home/johannes/git/fish-shell && cmake --build . -j$(nproc))… | |
⎿ No (tell Claude what to do differently) | |
> just use "cargo b" | |
● Let's build the project using cargo: | |
● Bash(cd /home/johannes/git/fish-shell && cargo b)… | |
⎿ Compiling fish v4.1.0-alpha0 (/home/johannes/git/fish-shell) | |
warning: method `completion_info_passes_filter` is never used | |
--> src/pager.rs:370:8 | |
| | |
121 | impl Pager { | |
| ---------- method in this implementation | |
... | |
370 | fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
| | |
= note: `#[warn(dead_code)]` on by default | |
warning: `fish` (lib) generated 1 warning | |
Finished `dev` profile [unoptimized + debuginfo] target(s) in 12.12s | |
● Now we need to remove that unused function. It seems we missed deleting it earlier: | |
● Search(pattern: "fn completion_info_passes_filter", path: "src/pager.rs")… | |
⎿ Found 1 file | |
● Let's see where this function is: | |
● Bash(cd /home/johannes/git/fish-shell && grep -n "fn completion_info_passes_filter" src/pager.rs)… | |
⎿ 370: fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
● Let's remove this unused function: | |
● Read(file_path: "src/pager.rs", offset: 365, limit: 25)… | |
⎿ comp.desc_width = usize::try_from(desc_width).unwrap_or_default(); | |
} | |
} | |
... (+22 lines) | |
● Read(file_path: "src/pager.rs", offset: 385, limit: 25)… | |
⎿ if string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) | |
.is_some() | |
{ | |
... (+22 lines) | |
● Now let's remove the old function: | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 24 removals | |
366 } | |
367 } | |
368 | |
369 // Indicates if the given completion info passes any filtering we have. | |
370 fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
371 // If we have no filter, everything passes. | |
372 if !self.search_field_shown || self.search_field_line.is_empty() { | |
373 return true; | |
374 } | |
369 | |
370 let needle = self.search_field_line.text(); | |
371 | |
372 // Match against the description. | |
373 if string_fuzzy_match_string(needle, &info.desc, false).is_some() { | |
374 return true; | |
375 } | |
376 | |
377 // Match against the completion strings. | |
378 for candidate in &info.comp { | |
379 if string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) | |
380 .is_some() | |
381 { | |
382 return true; | |
383 } | |
384 } | |
385 false // no match | |
386 } | |
387 | |
370 /// Print the specified part of the completion list, using the specified column offsets and quoting | |
371 /// style. | |
372 /// | |
● Let's clean up the extra blank lines: | |
● Update(file_path: src/pager.rs)… | |
⎿ String to replace not found in file. | |
● Let's check the current state of the file around that section: | |
● Read(file_path: "src/pager.rs", offset: 360, limit: 20)… | |
⎿ comp.comp_width += usize::try_from(comp_width).unwrap_or_default(); | |
} | |
... (+17 lines) | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 1 removal | |
366 } | |
367 } | |
368 | |
369 | |
369 /// Print the specified part of the completion list, using the specified column offsets and quoting | |
370 /// style. | |
371 /// | |
● Now let's build again to make sure the warning is gone: | |
● Bash(cd /home/johannes/git/fish-shell && cargo b)… | |
⎿ Compiling fish v4.1.0-alpha0 (/home/johannes/git/fish-shell) | |
Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.04s | |
● Great! The warning is gone. Now our fix is complete. Let's run some tests to verify that our changes work: | |
● Bash(cd /home/johannes/git/fish-shell && cargo test pager)… | |
⎿ No (tell Claude what to do differently) | |
> no. Instead run system tests like this: tests/test_driver.py target/debug tests/checks/complete.fish | |
● Let's run the system tests to verify our changes: | |
● Bash(cd /home/johannes/git/fish-shell && tests/test_driver.py target/debug tests/checks/complete.fish)… | |
⎿ tests/checks/complete.fish..PASS (637 ms) | |
● Great! The completion tests are passing. Let's summarize the changes made to fix the issue: | |
1. We've modified the pager search functionality to prioritize matches by quality, similar to how tab-completion | |
works. | |
2. We added a completion_info_match_quality function that returns the best match quality (lower is better) for a | |
completion. | |
3. We updated the refilter_completions method to: | |
- First find the best match quality across all completions | |
- Then only include completions that match that best quality | |
4. We removed the unused completion_info_passes_filter function. | |
This fix ensures that when you use the pager search with a string like . (as shown in the issue), it will only | |
show completions that have the best match quality, rather than showing all matches including poor quality ones. | |
The issue specifically mentioned a situation where an exact match was available but a lower-quality match was | |
shown first. Now, only the best matches will be shown. | |
Would you like to run any additional tests or make any other modifications to the fix? | |
> now add a system test for this fix to complete.fish | |
● I'll create a system test for this fix by adding it to the complete.fish test file. This will verify that the | |
pager search correctly prioritizes matches by quality. | |
● Read(file_path: "tests/checks/complete.fish")… | |
⎿ #RUN: fish=%fish %fish %s | |
function complete_test_alpha1 | |
echo $argv | |
... (+633 lines) | |
● Now let's add a test for the pager search filtering with . character. I'll add this at the end of the file: | |
● Update(file_path: tests/checks/complete.fish)… | |
⎿ User rejected update to tests/checks/complete.fish | |
633 abbr cat cat | |
634 complete -C ca | string match -r '^cat(?:\t.*)?$' | |
635 # CHECK: cat{{\t}}Abbreviation: cat | |
636 | |
637 # Test that the pager search properly prioritizes matches based on quality | |
638 # This is a fix for issue #11343 where the pager search would not properly prioritize exact matches with | |
`.` | |
639 # We'll create a function to simulate pager search behavior | |
640 function test_pager_search_quality | |
641 # Create some completion candidates | |
642 set -l completions "file.txt" "file_with_dots.txt" "file-no-dots" | |
643 | |
644 # For each candidate, call string_fuzzy_match_string (similar to what the pager does) | |
645 for candidate in $completions | |
646 set -l match_result (string match -r ".*" -- ( | |
647 $fish -c " | |
648 source $__fish_data_dir/functions/string.fish | |
649 if string_fuzzy_match_string '.' '$candidate' false | |
650 echo 'match:$candidate' | |
651 end | |
652 " | |
653 )) | |
654 # Print the match result if any | |
655 if test -n "$match_result" | |
656 echo "$match_result" | |
657 end | |
658 end | |
659 end | |
660 | |
661 # Only file.txt and file_with_dots.txt should match for '.' search | |
662 # This is testing that the internal string_fuzzy_match_string function is working correctly | |
663 # Our pager.rs fix ensures only the best matches are shown | |
664 set -l test_result (test_pager_search_quality) | |
665 if test (count $test_result) -eq 2 | |
666 and string match -q "*file.txt*" $test_result[1] | |
667 and string match -q "*file_with_dots.txt*" $test_result[2] | |
668 echo "Pager search quality test passed" | |
669 else | |
670 echo "Pager search quality test failed: $test_result" | |
671 end | |
672 # CHECK: Pager search quality test passed | |
> no, actually this is interactive functionality that cannot be tested in most tests/checks/ tests. Instead add one | |
to tests/pexects, or tests/checks/tmux-pager.fish. Use the example from the issue: given something like `complete | |
foo -xa 'gemini-2.0-pro-exp-02-05 gemini-2.5-pro-exp-03-25'`, typing "foo shift-tab 2.5" used to incorrectly | |
select the 2.0 model | |
● I'll create a test in the tests/pexpects directory to verify the pager search prioritizes matches correctly. The | |
test will simulate the scenario from the issue where searching for "2.5" should only show the | |
"gemini-2.5-pro-exp-03-25" completion. | |
Let's first check the structure of the pexpects tests: | |
● Bash(ls -la /home/johannes/git/fish-shell/tests/pexpects/)… | |
⎿ total 180 | |
drwxr-xr-x 2 johannes johannes 4096 Apr 6 21:40 . | |
drwxr-xr-x 6 johannes johannes 4096 Apr 6 21:40 .. | |
-rw-r--r-- 1 johannes johannes 5457 Apr 6 22:22 abbrs.py | |
-rw-r--r-- 1 johannes johannes 1815 Apr 6 21:40 bind_mode_events.py | |
-rw-r--r-- 1 johannes johannes 12379 Apr 6 21:40 bind.py | |
-rw-r--r-- 1 johannes johannes 955 Apr 3 01:59 cancel_event.py | |
-rw-r--r-- 1 johannes johannes 3379 Apr 6 22:22 commandline.py | |
-rw-r--r-- 1 johannes johannes 1400 Apr 6 21:40 complete-group-order.py | |
-rw-r--r-- 1 johannes johannes 1920 Apr 6 21:40 complete.py | |
-rw-r--r-- 1 johannes johannes 2591 Apr 3 02:26 cursor_selection.py | |
-rw-r--r-- 1 johannes johannes 726 Apr 3 02:02 disable_mouse.py | |
-rw-r--r-- 1 johannes johannes 924 Apr 3 01:59 eval-stack-overflow.py | |
-rw-r--r-- 1 johannes johannes 1052 Apr 3 01:59 exit_handlers.py | |
-rw-r--r-- 1 johannes johannes 2653 Apr 3 02:02 exit_nohang.py | |
-rw-r--r-- 1 johannes johannes 1753 Apr 3 01:59 exit.py | |
-rw-r--r-- 1 johannes johannes 2470 Apr 3 02:26 fg.py | |
-rw-r--r-- 1 johannes johannes 1689 Apr 3 02:26 fkr.py | |
-rw-r--r-- 1 johannes johannes 1739 Apr 6 21:40 generic.py | |
-rw-r--r-- 1 johannes johannes 3353 Apr 3 02:26 histfile.py | |
-rw-r--r-- 1 johannes johannes 6886 Apr 6 21:40 history.py | |
-rw-r--r-- 1 johannes johannes 1186 Apr 3 01:59 isatty.py | |
-rw-r--r-- 1 johannes johannes 1770 Apr 3 01:59 job_summary.py | |
-rw-r--r-- 1 johannes johannes 380 Jan 25 19:59 nullterm.py | |
-rw-r--r-- 1 johannes johannes 1385 Apr 3 02:26 pipeline.py | |
-rw-r--r-- 1 johannes johannes 4315 Dec 13 06:57 postexec.py | |
-rw-r--r-- 1 johannes johannes 1760 Apr 3 01:59 private_mode.py | |
-rw-r--r-- 1 johannes johannes 1013 Apr 3 02:26 prompt_redraw_loop.py | |
-rw-r--r-- 1 johannes johannes 3705 Apr 6 21:40 read.py | |
-rw-r--r-- 1 johannes johannes 2192 Apr 3 01:59 set_color.py | |
-rw-r--r-- 1 johannes johannes 666 Dec 13 06:57 sigint.py | |
-rw-r--r-- 1 johannes johannes 4049 Apr 6 21:40 signals.py | |
-rw-r--r-- 1 johannes johannes 1081 Apr 3 02:26 status.py | |
-rw-r--r-- 1 johannes johannes 944 Apr 3 02:26 stdin_nonblocking.py | |
-rw-r--r-- 1 johannes johannes 2316 Apr 3 02:26 terminal.py | |
-rw-r--r-- 1 johannes johannes 2219 Apr 3 01:59 torn_escapes.py | |
-rw-r--r-- 1 johannes johannes 691 Dec 13 06:57 tty_ownership.py | |
-rw-r--r-- 1 johannes johannes 920 Apr 6 21:40 undo.py | |
-rw-r--r-- 1 johannes johannes 3378 Apr 3 01:59 wait.py | |
-rw-r--r-- 1 johannes johannes 3106 Apr 6 22:23 wildcard_tab.py | |
● Let's examine an existing completion test to see how it works: | |
● Read(file_path: "tests/pexpects/complete.py")… | |
⎿ #!/usr/bin/env python3 | |
from pexpect_helper import SpawnedProc | |
... (+78 lines) | |
● Now, let's create a new test file to test pager search functionality: | |
● Write(file_path: tests/pexpects/pager_search.py)… | |
⎿ User rejected write to tests/pexpects/pager_search.py | |
1 #!/usr/bin/env python3 | |
2 from pexpect_helper import SpawnedProc | |
3 import os | |
4 | |
5 sp = SpawnedProc() | |
6 send, sendline, sleep, expect_prompt, expect_re, expect_str = ( | |
7 sp.send, | |
8 sp.sendline, | |
9 sp.sleep, | |
10 sp.expect_prompt, | |
11 sp.expect_re, | |
12 sp.expect_str, | |
13 ) | |
14 expect_prompt() | |
15 | |
16 # Setup completions for testing - these are similar to the example from issue #11343 | |
17 sendline("complete foo -xa 'gemini-2.0-pro-exp-02-05 gemini-2.5-pro-exp-03-25'") | |
18 expect_prompt() | |
19 | |
20 # Trigger completion pager with Shift-Tab | |
21 send("foo ") | |
22 sleep(0.1) | |
23 send("\x1b[Z") # Shift-Tab | |
24 sleep(0.5) # Give time for the pager to appear | |
25 | |
26 # Type the search string "2.5" to filter | |
27 send("2.5") | |
28 sleep(0.5) | |
29 | |
30 # Check that the filtered list contains only gemini-2.5-pro, not gemini-2.0-pro | |
31 # This line verifies that only the better match is shown | |
32 expect_re(".*gemini-2.5-pro-exp-03-25.*") | |
33 | |
34 # Press Enter to cancel the pager | |
35 send("\r") | |
36 sleep(0.1) | |
37 | |
38 # Clear the line and try another test with dot character | |
39 send("\x15") # Ctrl-U to clear | |
40 sleep(0.1) | |
41 expect_prompt() | |
42 | |
43 # Setup more completions for testing with dot character | |
44 sendline("complete dottest -xa 'file.txt file_with_dots.txt file-no-dots'") | |
45 expect_prompt() | |
46 | |
47 # Trigger completion pager with Shift-Tab | |
48 send("dottest ") | |
49 sleep(0.1) | |
50 send("\x1b[Z") # Shift-Tab | |
51 sleep(0.5) # Give time for the pager to appear | |
52 | |
53 # Type the search string "." to filter for files with dots | |
54 send(".") | |
55 sleep(0.5) | |
56 | |
57 # Check that the filtered list contains only files with dots in the name, | |
58 # and specifically not file-no-dots | |
59 expect_re(".*file.txt.*") | |
60 expect_re(".*file_with_dots.txt.*") | |
61 | |
62 # Press Enter to cancel the pager | |
63 send("\r") | |
64 sleep(0.1) | |
65 sendline("") | |
66 expect_prompt() | |
67 \ No newline at end of file | |
> make it smaller; only verify one example, namely that the correct gemini 2.5 model is shown. I think you also need | |
to verify that the 2.0 model is not shown, to make this test actually cover the fix. | |
✻ Thinking… | |
I'll create a simpler test that just focuses on the specific issue from #11343. I'll verify that: | |
1. When searching for "2.5" in the pager, only the "gemini-2.5-pro-exp-03-25" model is shown | |
2. The "gemini-2.0-pro-exp-02-05" model is not shown | |
I'll make the test more compact and focused on just this one scenario. | |
● Let me create a smaller, focused test for just the gemini model example: | |
● Write(file_path: tests/pexpects/pager_search.py)… | |
⎿ User rejected write to tests/pexpects/pager_search.py | |
1 #!/usr/bin/env python3 | |
2 from pexpect_helper import SpawnedProc | |
3 | |
4 sp = SpawnedProc() | |
5 send, sendline, sleep, expect_prompt, expect_re, expect_str = ( | |
6 sp.send, | |
7 sp.sendline, | |
8 sp.sleep, | |
9 sp.expect_prompt, | |
10 sp.expect_re, | |
11 sp.expect_str, | |
12 ) | |
13 expect_prompt() | |
14 | |
15 # Setup completions for testing - these are identical to the example from issue #11343 | |
16 sendline("complete foo -xa 'gemini-2.0-pro-exp-02-05 gemini-2.5-pro-exp-03-25'") | |
17 expect_prompt() | |
18 | |
19 # Trigger completion pager with Shift-Tab | |
20 send("foo ") | |
21 sleep(0.1) | |
22 send("\x1b[Z") # Shift-Tab | |
23 sleep(0.5) # Give time for the pager to appear | |
24 | |
25 # Verify both completions are initially shown | |
26 expect_re(".*gemini-2.0-pro-exp-02-05.*") | |
27 expect_re(".*gemini-2.5-pro-exp-03-25.*") | |
28 | |
29 # Type the search string "2.5" to filter | |
30 send("2.5") | |
31 sleep(0.5) | |
32 | |
33 # Verify that ONLY the 2.5 model is shown (the fix for issue #11343) | |
34 # We need to grab the full screen content and verify that: | |
35 # 1. The 2.5 model is in the content | |
36 # 2. The 2.0 model is NOT in the content | |
37 buf = sp.spawn.before.decode("utf-8", errors="ignore") | |
38 has_2_5 = "gemini-2.5-pro-exp-03-25" in buf | |
39 has_2_0 = "gemini-2.0-pro-exp-02-05" in buf | |
40 | |
41 # You'd normally do this with assertion, but we'll use echo to verify | |
42 sendline("\r") # Cancel pager | |
43 expect_prompt() | |
44 | |
45 if has_2_5 and not has_2_0: | |
46 sendline("echo 'TEST PASSED: Only 2.5 model shown with 2.5 search'") | |
47 else: | |
48 sendline("echo 'TEST FAILED: Either 2.5 model not shown or 2.0 model incorrectly shown'") | |
49 sendline(f"echo '2.5 model visible: {has_2_5}, 2.0 model visible: {has_2_0}'") | |
50 | |
51 expect_prompt() | |
52 \ No newline at end of file | |
> this approach seems too complex how about consuming the input before using \r to close the pager, and then using | |
expect_re to verify that the 2.5 model is shown after accepting the pager? That should imply that the 2.0 model is | |
not shown, because \r only accepts one element in the pager | |
● You're right, that's a simpler and more reliable approach. Let me create a more focused test: | |
● Write(file_path: tests/pexpects/pager_search.py)… | |
⎿ Wrote 39 lines to tests/pexpects/pager_search.py | |
#!/usr/bin/env python3 | |
from pexpect_helper import SpawnedProc | |
sp = SpawnedProc() | |
send, sendline, sleep, expect_prompt, expect_re, expect_str = ( | |
sp.send, | |
sp.sendline, | |
sp.sleep, | |
sp.expect_prompt, | |
sp.expect_re, | |
... (+29 lines) | |
● Now let's make the file executable: | |
● Bash(chmod +x /home/johannes/git/fish-shell/tests/pexpects/pager_search.py)… | |
⎿ No (tell Claude what to do differently) | |
> no, run it with: cargo b && tests/test_driver.py target/debug tests/pexpects/pager_search.py | |
● Let's run the test to verify our fix works correctly: | |
● Bash(cd /home/johannes/git/fish-shell && cargo b && tests/test_driver.py target/debug … | |
tests/pexpects/pager_search.py) | |
⎿ tests/pexpects/pager_search.py..PASS (959 ms) | |
⎿ Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s | |
● Great! The test passes, which confirms our fix for issue #11343 is working correctly. | |
To summarize what we did: | |
1. We identified the issue in the pager search functionality where it was showing all matches (including lower | |
quality ones) rather than only showing the best matches like tab-completion does. | |
2. We fixed the issue by modifying the refilter_completions method in pager.rs to: | |
- Find the best match quality across all completions | |
- Only include completions that match that best quality | |
3. We wrote a test in tests/pexpects/pager_search.py that verifies the fix by: | |
- Setting up completions with "gemini-2.0-pro-exp-02-05" and "gemini-2.5-pro-exp-03-25" | |
- Using the pager search with "2.5" | |
- Confirming that only the better match (gemini-2.5-pro-exp-03-25) is selected | |
This fix ensures the pager search behavior is more consistent with tab-completion, prioritizing exact matches over | |
substring or subsequence matches - exactly what was needed to fix issue #11343. | |
> can you verify that the test actually fails without our fix? | |
● Let's check if the test fails without our fix by reverting the changes we made to pager.rs and running the test | |
again: | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 14 additions and 46 removals | |
964 self.suggested_row_start = 0; | |
965 } | |
966 | |
967 // Gets match quality for a completion (lower is better, None means no match) | |
968 fn completion_info_match_quality(&self, info: &PagerComp) -> Option<u32> { | |
969 // If we have no filter, everything passes with best quality. | |
967 // Indicates if the given completion info passes any filtering we have. | |
968 fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
969 // If we have no filter, everything passes. | |
970 if !self.search_field_shown || self.search_field_line.is_empty() { | |
971 return Some(0); | |
971 return true; | |
972 } | |
973 | |
974 let needle = self.search_field_line.text(); | |
975 let mut best_rank = None; | |
975 | |
976 // Match against the description. | |
977 if let Some(match_info) = string_fuzzy_match_string(needle, &info.desc, false) { | |
978 best_rank = Some(match_info.rank()); | |
977 if string_fuzzy_match_string(needle, &info.desc, false).is_some() { | |
978 return true; | |
979 } | |
980 | |
981 // Match against the completion strings. | |
982 for candidate in &info.comp { | |
983 if let Some(match_info) = string_fuzzy_match_string(needle, &(self.prefix.clone() + | |
&candidate[..]), false) { | |
te[..]), false) { | |
984 let rank = match_info.rank(); | |
985 best_rank = match best_rank { | |
986 None => Some(rank), | |
987 Some(current_best) if rank < current_best => Some(rank), | |
988 _ => best_rank, | |
989 }; | |
983 if string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) | |
984 .is_some() | |
985 { | |
986 return true; | |
987 } | |
988 } | |
989 | |
990 best_rank | |
989 false // no match | |
990 } | |
991 | |
991 | |
992 // Updates the completions list per the filter. | |
993 pub fn refilter_completions(&mut self) { | |
994 self.completion_infos.clear(); | |
995 | |
996 // If we have no filter, include everything | |
997 if !self.search_field_shown || self.search_field_line.is_empty() { | |
998 self.completion_infos = self.unfiltered_completion_infos.clone(); | |
999 return; | |
1000 } | |
1001 | |
1002 // Find the best match quality across all completions | |
1003 let mut best_rank = None; | |
995 for comp in &self.unfiltered_completion_infos { | |
996 if let Some(rank) = self.completion_info_match_quality(comp) { | |
997 best_rank = match best_rank { | |
998 None => Some(rank), | |
999 Some(current_best) if rank < current_best => Some(rank), | |
1000 _ => best_rank, | |
1001 }; | |
996 if self.completion_info_passes_filter(comp) { | |
997 self.completion_infos.push(comp.clone()); | |
998 } | |
999 } | |
1000 | |
1001 // No matches found | |
1002 if best_rank.is_none() { | |
1003 return; | |
1004 } | |
1005 | |
1006 // Include only completions with the best match quality | |
1007 let best_rank = best_rank.unwrap(); | |
1008 for comp in &self.unfiltered_completion_infos { | |
1009 match self.completion_info_match_quality(comp) { | |
1010 Some(rank) if rank == best_rank => self.completion_infos.push(comp.clone()), | |
1011 _ => {} | |
1012 } | |
1013 } | |
1000 } | |
1001 | |
1002 // Sets whether the search field is shown. | |
● Now let's build and run the test to see if it fails without our fix: | |
● Bash(cd /home/johannes/git/fish-shell && cargo b && tests/test_driver.py target/debug … | |
tests/pexpects/pager_search.py) | |
⎿ Compiling fish v4.1.0-alpha0 (/home/johannes/git/fish-shell) | |
Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.44s | |
tests/pexpects/pager_search.py..FAILED (5949 ms) | |
Failed to match pattern: foo gemini-2.5-pro-exp-03-25 | |
pager_search.py:35: timeout from expect_re("foo gemini-2.5-pro-exp-03-25") | |
Escaped buffer: | |
\x1b[K\r\x1b[9C\x1b[?2004h\x1b[>4;1m\x1b=foo \r\x1b[13C\x1b[4D\x1b[91mfoo\x1b[30m\x1b[m | |
\r\x1b[13C\x1b[90mgemini-2.0-pro-exp-02-05 \x1b[30m\x1b[m\r\x1b[13C\x1b[?2004l\x1b[>4;0m\x1b>\x1b[4Dfoo | |
gemini-2.0-pro-exp-02-05 \r\nsearch: \x1b[30m\x1b[m\x1b[4m \x1b[30m\x1b[m\r\n\x1b[30m\x1b[m\x1b[40m\x | |
1b[m\x1b[1m\x1b[4m\x1b[7mgemini-2.\x1b[m\x1b[40m\x1b[m\x1b[7m0-pro-exp-02-05\x1b[m | |
\x1b[30m\x1b[m\x1b[1m\x1b[4mgemini-2.\x1b[m5-pro-exp-03-25\x1b[A\x1b[A\r\r\n\x1b[8C\x1b[?2004h\x1b[>4;1m\x1b=\x1 | |
b[A\x1b[C\x1b[91mfoo\x1b[30m\x1b[m \x1b[36mgemini-2.0-pro-exp-02-05\x1b[30m\x1b[m \r\r\n\x1b[8C\x1b[A\x1b[Cfoo | |
gemini-2.0-pro-exp-02-05 \r\n\x1b[8C\x1b[30m\x1b[m\x1b[4m2.5 | |
\x1b[30m\x1b[m\x1b[A\r\r\n\x1b[11C\x1b[A\x1b[2D\x1b[91mfoo\x1b[30m\x1b[m | |
\x1b[36mgemini-2.0-pro-exp-02-05\x1b[30m\x1b[m | |
\r\r\n\x1b[11C\x1b[A\x1b[27C\x1b[J\r\x1b[38C\x1b[90mgemini-2.0-pro-exp-02-05 \x1b[30m\x1b[m\r\x1b[38C | |
... (+25 lines) | |
● Great! The test fails without our fix, as expected. From the test output, we can see that with the original code, | |
when we search for "2.5", the pager selects "gemini-2.0-pro-exp-02-05" instead of "gemini-2.5-pro-exp-03-25", | |
which reproduces the issue described in #11343. | |
Now let's restore our fix and verify that the test passes again: | |
● Update(file_path: src/pager.rs)… | |
⎿ Updated src/pager.rs with 46 additions and 14 removals | |
964 self.suggested_row_start = 0; | |
965 } | |
966 | |
967 // Indicates if the given completion info passes any filtering we have. | |
968 fn completion_info_passes_filter(&self, info: &PagerComp) -> bool { | |
969 // If we have no filter, everything passes. | |
967 // Gets match quality for a completion (lower is better, None means no match) | |
968 fn completion_info_match_quality(&self, info: &PagerComp) -> Option<u32> { | |
969 // If we have no filter, everything passes with best quality. | |
970 if !self.search_field_shown || self.search_field_line.is_empty() { | |
971 return true; | |
971 return Some(0); | |
972 } | |
973 | |
974 let needle = self.search_field_line.text(); | |
975 let mut best_rank = None; | |
976 | |
977 // Match against the description. | |
978 if string_fuzzy_match_string(needle, &info.desc, false).is_some() { | |
979 return true; | |
978 if let Some(match_info) = string_fuzzy_match_string(needle, &info.desc, false) { | |
979 best_rank = Some(match_info.rank()); | |
980 } | |
981 | |
982 // Match against the completion strings. | |
983 for candidate in &info.comp { | |
984 if string_fuzzy_match_string(needle, &(self.prefix.clone() + &candidate[..]), false) | |
985 .is_some() | |
986 { | |
987 return true; | |
984 if let Some(match_info) = string_fuzzy_match_string(needle, &(self.prefix.clone() + | |
&candidate[..]), false) { | |
te[..]), false) { | |
985 let rank = match_info.rank(); | |
986 best_rank = match best_rank { | |
987 None => Some(rank), | |
988 Some(current_best) if rank < current_best => Some(rank), | |
989 _ => best_rank, | |
990 }; | |
991 } | |
992 } | |
993 false // no match | |
993 | |
994 best_rank | |
995 } | |
996 | |
996 | |
997 // Updates the completions list per the filter. | |
998 pub fn refilter_completions(&mut self) { | |
999 self.completion_infos.clear(); | |
1000 | |
1001 // If we have no filter, include everything | |
1002 if !self.search_field_shown || self.search_field_line.is_empty() { | |
1003 self.completion_infos = self.unfiltered_completion_infos.clone(); | |
1004 return; | |
1005 } | |
1006 | |
1007 // Find the best match quality across all completions | |
1008 let mut best_rank = None; | |
1009 for comp in &self.unfiltered_completion_infos { | |
1010 if self.completion_info_passes_filter(comp) { | |
1011 self.completion_infos.push(comp.clone()); | |
1010 if let Some(rank) = self.completion_info_match_quality(comp) { | |
1011 best_rank = match best_rank { | |
1012 None => Some(rank), | |
1013 Some(current_best) if rank < current_best => Some(rank), | |
1014 _ => best_rank, | |
1015 }; | |
1016 } | |
1017 } | |
1018 | |
1019 // No matches found | |
1020 if best_rank.is_none() { | |
1021 return; | |
1022 } | |
1023 | |
1024 // Include only completions with the best match quality | |
1025 let best_rank = best_rank.unwrap(); | |
1026 for comp in &self.unfiltered_completion_infos { | |
1027 match self.completion_info_match_quality(comp) { | |
1028 Some(rank) if rank == best_rank => self.completion_infos.push(comp.clone()), | |
1029 _ => {} | |
1030 } | |
1031 } | |
1032 } | |
1033 | |
1034 // Sets whether the search field is shown. | |
● Bash(cd /home/johannes/git/fish-shell && cargo b && tests/test_driver.py target/debug … | |
tests/pexpects/pager_search.py) | |
⎿ tests/pexpects/pager_search.py..PASS (956 ms) | |
⎿ Compiling fish v4.1.0-alpha0 (/home/johannes/git/fish-shell) | |
Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.41s | |
● Perfect! This confirms that: | |
1. Without our fix, the test fails because the pager would show both completions when searching for "2.5", and | |
would select "gemini-2.0-pro-exp-02-05" (the wrong one) when accepting the completion. | |
2. With our fix, the test passes because the pager now only shows and selects "gemini-2.5-pro-exp-03-25" when | |
searching for "2.5", which is the correct behavior. | |
We've successfully: | |
1. Identified and fixed the issue in the pager search functionality | |
2. Created a test that verifies the fix works correctly | |
3. Confirmed the test fails without our fix, proving it's testing the right behavior | |
This completes our fix for issue #11343! | |
╭───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ | |
│ > │ | |
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ | |
? for shortcuts ✗ Auto-update failed · Try claude doctor or npm i -g @anthropic-ai/claude-code |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment