mirror of
https://github.com/fish-shell/fish-shell.git
synced 2026-04-19 06:31:13 -03:00
Fix prefix/suffix icase comparisons
As reported on Gitter, running "echo İ" makes history autosuggestion
for "echo i" crash. This is because history search correctly
returns the former, but string_prefixes_string_case_insensitive("i",
"İ") incorrectly returns false. This is because the prefix check
is implemented by trimming the rhs to the length of the prefix and
checking if the result is equal to the prefix. This is wrong because
the prefix computation should operate on the canonical lowercase
version, because that's what history search uses.
This commit is contained in:
@@ -67,6 +67,7 @@ For distributors and developers
|
||||
Regression fixes:
|
||||
-----------------
|
||||
- (from 4.1.0) Crash on incorrectly-set color variables (:issue:`12078`).
|
||||
- (from 4.1.0) Crash when autosuggesting Unicode characters with nontrivial lowercase mapping.
|
||||
- (from 4.2.0) Incorrect emoji width computation on macOS.
|
||||
- (from 4.2.0) Mouse clicks and :kbd:`ctrl-l` edge cases in multiline command lines (:issue:`12121`).
|
||||
- (from 4.2.0) Completions for Git remote names on some non-glibc systems.
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
//!
|
||||
//! Many of these functions are more or less broken and incomplete.
|
||||
|
||||
use fish_wchar::{CharsUtf32, prelude::*};
|
||||
use fish_wchar::prelude::*;
|
||||
use fish_widecharwidth::{WcLookupTable, WcWidth};
|
||||
use once_cell::sync::Lazy;
|
||||
use std::cmp;
|
||||
@@ -126,21 +126,26 @@ pub fn wcscasecmp_fuzzy(
|
||||
.cmp(lowercase(rhs.chars()).map(extra_canonicalization))
|
||||
}
|
||||
|
||||
pub fn lowercase(chars: CharsUtf32) -> impl Iterator<Item = char> {
|
||||
use std::char::ToLowercase;
|
||||
use widestring::utfstr::CharsUtf32;
|
||||
|
||||
/// This struct streams the underlying lowercase chars of a `UTF32String` without allocating.
|
||||
///
|
||||
/// `char::to_lowercase()` returns an iterator of chars and we sometimes need to cmp the last
|
||||
/// char of one char's `to_lowercase()` with the first char of the other char's
|
||||
/// `to_lowercase()`. This makes that possible.
|
||||
struct ToLowerBuffer<'a> {
|
||||
pub fn lowercase(chars: impl Iterator<Item = char>) -> impl Iterator<Item = char> {
|
||||
lowercase_impl(chars, |c| c.to_lowercase())
|
||||
}
|
||||
pub fn lowercase_rev(chars: impl DoubleEndedIterator<Item = char>) -> impl Iterator<Item = char> {
|
||||
lowercase_impl(chars.rev(), |c| c.to_lowercase().rev())
|
||||
}
|
||||
fn lowercase_impl<ToLowercase: Iterator<Item = char>>(
|
||||
chars: impl Iterator<Item = char>,
|
||||
to_lowercase: fn(char) -> ToLowercase,
|
||||
) -> impl Iterator<Item = char> {
|
||||
/// This struct streams the underlying lowercase chars of a string without allocating.
|
||||
struct ToLowerBuffer<Chars: Iterator<Item = char>, ToLowercase: Iterator<Item = char>> {
|
||||
to_lowercase: fn(char) -> ToLowercase,
|
||||
current: ToLowercase,
|
||||
chars: CharsUtf32<'a>,
|
||||
chars: Chars,
|
||||
}
|
||||
|
||||
impl<'a> Iterator for ToLowerBuffer<'a> {
|
||||
impl<Chars: Iterator<Item = char>, ToLowercase: Iterator<Item = char>> Iterator
|
||||
for ToLowerBuffer<Chars, ToLowercase>
|
||||
{
|
||||
type Item = char;
|
||||
|
||||
fn next(&mut self) -> Option<Self::Item> {
|
||||
@@ -148,29 +153,31 @@ fn next(&mut self) -> Option<Self::Item> {
|
||||
return Some(c);
|
||||
}
|
||||
|
||||
self.current = self.chars.next()?.to_lowercase();
|
||||
self.current = (self.to_lowercase)(self.chars.next()?);
|
||||
self.current.next()
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> ToLowerBuffer<'a> {
|
||||
pub fn new(mut chars: CharsUtf32<'a>) -> Self {
|
||||
impl<Chars: Iterator<Item = char>, ToLowercase: Iterator<Item = char>>
|
||||
ToLowerBuffer<Chars, ToLowercase>
|
||||
{
|
||||
pub fn new(mut chars: Chars, to_lowercase: fn(char) -> ToLowercase) -> Self {
|
||||
Self {
|
||||
to_lowercase,
|
||||
current: chars.next().map_or_else(
|
||||
|| {
|
||||
let mut empty = 'a'.to_lowercase();
|
||||
let mut empty = to_lowercase('a');
|
||||
let _ = empty.next();
|
||||
debug_assert!(empty.next().is_none());
|
||||
empty
|
||||
},
|
||||
|c| c.to_lowercase(),
|
||||
to_lowercase,
|
||||
),
|
||||
chars,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ToLowerBuffer::new(chars)
|
||||
ToLowerBuffer::new(chars, to_lowercase)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
use crate::common::{get_ellipsis_char, get_ellipsis_str};
|
||||
use crate::prelude::*;
|
||||
use fish_fallback::{fish_wcwidth, wcscasecmp, wcscasecmp_fuzzy};
|
||||
use fish_fallback::{fish_wcwidth, lowercase, lowercase_rev, wcscasecmp, wcscasecmp_fuzzy};
|
||||
use fish_wchar::decode_byte_from_char;
|
||||
|
||||
/// Return the number of newlines in a string.
|
||||
@@ -21,8 +21,9 @@ pub fn count_newlines(s: &wstr) -> usize {
|
||||
|
||||
/// Test if a string prefixes another without regard to case. Returns true if a is a prefix of b.
|
||||
pub fn string_prefixes_string_case_insensitive(proposed_prefix: &wstr, value: &wstr) -> bool {
|
||||
let prefix_size = proposed_prefix.len();
|
||||
prefix_size <= value.len() && wcscasecmp(&value[..prefix_size], proposed_prefix).is_eq()
|
||||
let mut proposed_prefix = lowercase(proposed_prefix.chars());
|
||||
let value = lowercase(value.chars());
|
||||
proposed_prefix.by_ref().zip(value).all(|(a, b)| a == b) && proposed_prefix.next().is_none()
|
||||
}
|
||||
|
||||
pub fn string_prefixes_string_maybe_case_insensitive(
|
||||
@@ -47,9 +48,9 @@ pub fn strip_executable_suffix(path: &wstr) -> Option<&wstr> {
|
||||
|
||||
/// Test if a string is a suffix of another.
|
||||
pub fn string_suffixes_string_case_insensitive(proposed_suffix: &wstr, value: &wstr) -> bool {
|
||||
let suffix_size = proposed_suffix.len();
|
||||
suffix_size <= value.len()
|
||||
&& wcscasecmp(&value[value.len() - suffix_size..], proposed_suffix).is_eq()
|
||||
let mut proposed_suffix = lowercase_rev(proposed_suffix.chars());
|
||||
let value = lowercase_rev(value.chars());
|
||||
proposed_suffix.by_ref().zip(value).all(|(a, b)| a == b) && proposed_suffix.next().is_none()
|
||||
}
|
||||
|
||||
/// Test if a string prefixes another. Returns true if a is a prefix of b.
|
||||
@@ -553,10 +554,44 @@ pub fn fish_wcwidth_visible(c: char) -> isize {
|
||||
mod tests {
|
||||
use super::{
|
||||
CaseSensitivity, ContainType, LineIterator, count_newlines, ifind, join_strings,
|
||||
split_string_tok, string_fuzzy_match_string,
|
||||
split_string_tok, string_fuzzy_match_string, string_prefixes_string_case_insensitive,
|
||||
string_suffixes_string_case_insensitive,
|
||||
};
|
||||
use crate::prelude::*;
|
||||
|
||||
#[test]
|
||||
fn test_string_prefixes_string_case_insensitive() {
|
||||
macro_rules! validate {
|
||||
($prefix:literal, $s:literal, $expected:expr) => {
|
||||
assert_eq!(
|
||||
string_prefixes_string_case_insensitive(L!($prefix), L!($s)),
|
||||
$expected
|
||||
);
|
||||
};
|
||||
}
|
||||
validate!("i", "i_", true);
|
||||
validate!("İ", "i\u{307}_", true);
|
||||
validate!("i\u{307}", "İ", true); // prefix is longer
|
||||
validate!("i", "İ", true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_string_suffixes_string_case_insensitive() {
|
||||
macro_rules! validate {
|
||||
($suffix:literal, $s:literal, $expected:expr) => {
|
||||
assert_eq!(
|
||||
string_suffixes_string_case_insensitive(L!($suffix), L!($s)),
|
||||
$expected
|
||||
);
|
||||
};
|
||||
}
|
||||
validate!("i", "_i", true);
|
||||
validate!("i\u{307}", "İ", true);
|
||||
validate!("İ", "i\u{307}", true); // suffix is longer
|
||||
validate!("İ", "_İ", true);
|
||||
validate!("i", "_İ", false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_ifind() {
|
||||
macro_rules! validate {
|
||||
|
||||
@@ -45,3 +45,8 @@ isolated-tmux send-keys C-l 'echo ('
|
||||
tmux-sleep
|
||||
isolated-tmux capture-pane -p
|
||||
# CHECK: prompt {{\d+}}> echo (echo)
|
||||
|
||||
isolated-tmux send-keys C-u 'echo İ___' Enter C-l 'echo i'
|
||||
tmux-sleep
|
||||
isolated-tmux capture-pane -p
|
||||
# CHECK: prompt {{\d+}}> echo İ___
|
||||
|
||||
Reference in New Issue
Block a user