From 02bd2b4bcd6fdea085d829150d078ef04efc4b2f Mon Sep 17 00:00:00 2001 From: alesito85 Date: Wed, 15 Feb 2023 13:20:20 +0100 Subject: [PATCH 1/3] Attempt at fixing find_common_string --- src/menu/menu_functions.rs | 49 +++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/menu/menu_functions.rs b/src/menu/menu_functions.rs index 982235bf..89f7fb56 100644 --- a/src/menu/menu_functions.rs +++ b/src/menu/menu_functions.rs @@ -147,29 +147,27 @@ pub fn find_common_string(values: &[Suggestion]) -> (Option<&Suggestion>, Option let index = first.and_then(|first| { values.iter().skip(1).fold(None, |index, suggestion| { - if suggestion.value.starts_with(&first.value) { - Some(first.value.len()) - } else { - first - .value - .char_indices() - .zip(suggestion.value.char_indices()) - .find(|((_, mut lhs), (_, mut rhs))| { - lhs.make_ascii_lowercase(); - rhs.make_ascii_lowercase(); - - lhs != rhs - }) - .map(|((new_index, _), _)| match index { - Some(index) => { - if index <= new_index { - index - } else { - new_index - } - } - None => new_index, - }) + let x = first + .value + .char_indices() + .zip(suggestion.value.char_indices()) + .find(|((_, mut lhs), (_, mut rhs))| { + lhs.make_ascii_lowercase(); + rhs.make_ascii_lowercase(); + + lhs != rhs + }); + + match x { + Some((lhs, rhs)) => { + let (lhsi, _) = lhs; + let (rhsi, _) = rhs; + match index { + Some(_) => index.min(Some(lhsi)).min(Some(rhsi)), + None => Some(lhsi.min(rhsi)) + } + }, + None => index } }) }); @@ -451,7 +449,7 @@ mod tests { fn find_common_string_with_ansi() { use crate::Span; - let input: Vec<_> = ["nushell", "null"] + let input: Vec<_> = ["qml", "qmake", "qmltc", "qmlls", "qmldom", "qmake6", "qmltime", "qmllint", "qmlscene", "qmlformat", "qmleasing", "qmlpreview", "qmlprofiler", "qmlplugindump", "qmltestrunner"] .into_iter() .map(|s| Suggestion { value: s.into(), @@ -463,7 +461,8 @@ mod tests { .collect(); let res = find_common_string(&input); - assert!(matches!(res, (Some(elem), Some(2)) if elem == &input[0])); + assert!(matches!(res, (Some(elem), Some(2)) + if elem.value.chars().take(2).collect::() == input[0].value.chars().take(2).collect::())); } #[test] From cc4dbcdcf7252b484ac152b7f7f6da4785204190 Mon Sep 17 00:00:00 2001 From: alesito85 Date: Wed, 15 Feb 2023 15:17:27 +0100 Subject: [PATCH 2/3] Formatting fix --- src/menu/menu_functions.rs | 42 ++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/menu/menu_functions.rs b/src/menu/menu_functions.rs index 89f7fb56..544fa4b5 100644 --- a/src/menu/menu_functions.rs +++ b/src/menu/menu_functions.rs @@ -164,10 +164,10 @@ pub fn find_common_string(values: &[Suggestion]) -> (Option<&Suggestion>, Option let (rhsi, _) = rhs; match index { Some(_) => index.min(Some(lhsi)).min(Some(rhsi)), - None => Some(lhsi.min(rhsi)) + None => Some(lhsi.min(rhsi)), } - }, - None => index + } + None => index, } }) }); @@ -449,16 +449,32 @@ mod tests { fn find_common_string_with_ansi() { use crate::Span; - let input: Vec<_> = ["qml", "qmake", "qmltc", "qmlls", "qmldom", "qmake6", "qmltime", "qmllint", "qmlscene", "qmlformat", "qmleasing", "qmlpreview", "qmlprofiler", "qmlplugindump", "qmltestrunner"] - .into_iter() - .map(|s| Suggestion { - value: s.into(), - description: None, - extra: None, - span: Span::new(0, s.len()), - append_whitespace: false, - }) - .collect(); + let input: Vec<_> = [ + "qml", + "qmake", + "qmltc", + "qmlls", + "qmldom", + "qmake6", + "qmltime", + "qmllint", + "qmlscene", + "qmlformat", + "qmleasing", + "qmlpreview", + "qmlprofiler", + "qmlplugindump", + "qmltestrunner", + ] + .into_iter() + .map(|s| Suggestion { + value: s.into(), + description: None, + extra: None, + span: Span::new(0, s.len()), + append_whitespace: false, + }) + .collect(); let res = find_common_string(&input); assert!(matches!(res, (Some(elem), Some(2)) From 70e9efe91914bbaeda83ebdb786e809d6eecf914 Mon Sep 17 00:00:00 2001 From: alesito85 Date: Tue, 21 Feb 2023 21:37:54 +0100 Subject: [PATCH 3/3] Expanded functionality and tests against empty haystack and others --- src/menu/menu_functions.rs | 193 ++++++++++++++++++++++++++++++------- 1 file changed, 157 insertions(+), 36 deletions(-) diff --git a/src/menu/menu_functions.rs b/src/menu/menu_functions.rs index 544fa4b5..1838b200 100644 --- a/src/menu/menu_functions.rs +++ b/src/menu/menu_functions.rs @@ -145,31 +145,54 @@ pub fn parse_selection_char(buffer: &str, marker: char) -> ParseResult { pub fn find_common_string(values: &[Suggestion]) -> (Option<&Suggestion>, Option) { let first = values.iter().next(); + let mut new_index: Option = match first { + Some(first) => Some(first.value.len()), + None => Some(0), + }; + + if values.len() <= 1 { + return (first, new_index); + } + let index = first.and_then(|first| { - values.iter().skip(1).fold(None, |index, suggestion| { - let x = first - .value - .char_indices() - .zip(suggestion.value.char_indices()) - .find(|((_, mut lhs), (_, mut rhs))| { - lhs.make_ascii_lowercase(); - rhs.make_ascii_lowercase(); - - lhs != rhs - }); - - match x { - Some((lhs, rhs)) => { - let (lhsi, _) = lhs; - let (rhsi, _) = rhs; - match index { - Some(_) => index.min(Some(lhsi)).min(Some(rhsi)), - None => Some(lhsi.min(rhsi)), - } + let vals = values + .iter() + .skip(1) + .try_fold(None, |index: Option, suggestion| { + if first.value.get(..1) != suggestion.value.get(..1) { + return Err(0); } - None => index, - } - }) + let tmp_index = first + .value + .char_indices() + .zip(suggestion.value.char_indices()) + .find(|((_, mut lhs), (_, mut rhs))| { + lhs.make_ascii_lowercase(); + rhs.make_ascii_lowercase(); + + lhs != rhs + }) + .map(|((new_index, _), _)| match index { + Some(index) => { + if index <= new_index { + index + } else { + new_index + } + } + None => new_index, + }); + + if tmp_index > Some(0) { + new_index = tmp_index; + } + Ok(new_index) + }); + + match vals { + Ok(index) => index, + Err(_) => Some(0), + } }); (first, index) @@ -248,6 +271,8 @@ pub fn string_difference<'a>(new_string: &'a str, old_string: &str) -> (usize, & #[cfg(test)] mod tests { + use itertools::Itertools; + use super::*; #[test] @@ -449,7 +474,7 @@ mod tests { fn find_common_string_with_ansi() { use crate::Span; - let input: Vec<_> = [ + let source = [ "qml", "qmake", "qmltc", @@ -465,20 +490,116 @@ mod tests { "qmlprofiler", "qmlplugindump", "qmltestrunner", - ] - .into_iter() - .map(|s| Suggestion { - value: s.into(), - description: None, - extra: None, - span: Span::new(0, s.len()), - append_whitespace: false, - }) - .collect(); + ]; + // Test against empty haystack + let input: Vec<_> = source + .into_iter() + .take(0) + .map(|s: &str| Suggestion { + value: s.into(), + description: None, + extra: None, + span: Span::new(0, s.len()), + append_whitespace: false, + }) + .collect(); + let res = find_common_string(&input); + + match res.0 { + Some(_) => assert!(false, "There shouldn't be any elements"), + None => assert!(res.1 == Some(0)), + } + + // Test against haystack of size 1 + let input: Vec<_> = source + .into_iter() + .take(1) + .map(|s: &str| Suggestion { + value: s.into(), + description: None, + extra: None, + span: Span::new(0, s.len()), + append_whitespace: false, + }) + .collect(); let res = find_common_string(&input); - assert!(matches!(res, (Some(elem), Some(2)) - if elem.value.chars().take(2).collect::() == input[0].value.chars().take(2).collect::())); + match res { + (Some(first), Some(index)) => assert!( + first.value == input.first().unwrap().value && index == first.value.len()), + _ => assert!(false, "There should be a result with both options as some with one element to check against"), + } + + // Test against entire haystack that's originally errored + let input: Vec<_> = source + .into_iter() + .map(|s: &str| Suggestion { + value: s.into(), + description: None, + extra: None, + span: Span::new(0, s.len()), + append_whitespace: false, + }) + .collect(); + let res = find_common_string(&input); + + match res { + (Some(first), Some(index)) => { + assert!(res.1 == Some(2), "Index check failed for full haystack"); + input.iter().for_each(|suggestion| { + assert!(first.value.chars().take(index - 1).collect::() == suggestion.value.chars().take(index - 1).collect::()) + }) + }, + _ => assert!(false, "There should be a result with both options as some with one element to check against for full haystack"), + } + + // Test against reduced haystack + let input: Vec<_> = source + .into_iter() + .filter(|i| i.starts_with("qma")) + .collect_vec() + .into_iter() + .map(|s: &str| Suggestion { + value: s.into(), + description: None, + extra: None, + span: Span::new(0, s.len()), + append_whitespace: false, + }) + .into_iter() + .collect(); + let res = find_common_string(&input); + + match res { + (Some(first), Some(index)) => { + assert!(res.1 == Some(5), "Index check failed for haystack of 'qma*'"); + input.iter().for_each(|suggestion| { + assert!(first.value.chars().take(index - 1).collect::() == suggestion.value.chars().take(index - 1).collect::()) + }) + }, + _ => assert!(false, "There should be a result with both options as some with one element to check against for reduced haystack"), + } + + // Test against reduced haystack that differs at first character + let input: Vec<_> = ["qma", "lsf"] + .into_iter() + .map(|s: &str| Suggestion { + value: s.into(), + description: None, + extra: None, + span: Span::new(0, s.len()), + append_whitespace: false, + }) + .into_iter() + .collect(); + let res = find_common_string(&input); + + match res { + (Some(_), Some(_)) => { + assert!(res.1 == Some(0), "Index check failed for haystack of different first chars"); + }, + _ => assert!(false, "There should be a result with both options as some with one element to check against for different first chars"), + } } #[test]