From a2bc28c010c671c6c0cad97f93d4eccc69944ddc Mon Sep 17 00:00:00 2001 From: sholderbach Date: Tue, 26 Mar 2024 14:55:46 +0100 Subject: [PATCH 1/4] Fix case-consistency searching sqlite history For the `FileBackedHistory` those operations have always been case sensitive, do the same for `SqliteBackedHistory`. The insensitivity of `like` in sqlite causes https://github.com/nushell/nushell/issues/10131 For substring matching for now use `glob` instead of `like`, this changes the wildcard from `%` to `*` which is more common in the Nushell context. We have so far not been performing proper escaping here. User queries may match more often in surprising ways. `Exact` should now be exact. --- src/history/sqlite_backed.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/history/sqlite_backed.rs b/src/history/sqlite_backed.rs index ec3b021a..9f3e6fd8 100644 --- a/src/history/sqlite_backed.rs +++ b/src/history/sqlite_backed.rs @@ -329,10 +329,10 @@ impl SqliteBackedHistory { // TODO: escape % let command_line_like = match command_line { CommandLineSearch::Exact(e) => e.to_string(), - CommandLineSearch::Prefix(prefix) => format!("{prefix}%"), - CommandLineSearch::Substring(cont) => format!("%{cont}%"), + CommandLineSearch::Prefix(prefix) => format!("{prefix}*"), + CommandLineSearch::Substring(cont) => format!("*{cont}*"), }; - wheres.push("command_line like :command_line"); + wheres.push("command_line glob :command_line"); params.push((":command_line", Box::new(command_line_like))); } From a4a65d8829b5fea6bdb19b0b61d1e232b869f133 Mon Sep 17 00:00:00 2001 From: sholderbach Date: Tue, 26 Mar 2024 15:19:50 +0100 Subject: [PATCH 2/4] Add test for case-sensitive prefix search Link the relevant issue so feature fans don't reintroduce bugs --- src/history/base.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/history/base.rs b/src/history/base.rs index 4c07417b..a7c56f6e 100644 --- a/src/history/base.rs +++ b/src/history/base.rs @@ -349,6 +349,24 @@ mod test { Ok(()) } + #[test] + fn search_prefix_is_case_sensitive() -> Result<()> { + // Basic prefix search should preserve case + // + // https://github.com/nushell/nushell/issues/10131 + let history = create_filled_example_history()?; + let res = history.search(SearchQuery { + filter: SearchFilter::from_text_search( + CommandLineSearch::Prefix("LS ".to_string()), + None, + ), + ..SearchQuery::everything(SearchDirection::Backward, None) + })?; + search_returned(&*history, res, vec![])?; + + Ok(()) + } + #[test] fn search_includes() -> Result<()> { let history = create_filled_example_history()?; From c5b987cc19e8f348e3973ca3d2dfa361cabcc9e0 Mon Sep 17 00:00:00 2001 From: sholderbach Date: Tue, 26 Mar 2024 15:20:25 +0100 Subject: [PATCH 3/4] Use sqlite `instr` function for case-exact match --- src/history/sqlite_backed.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/history/sqlite_backed.rs b/src/history/sqlite_backed.rs index 9f3e6fd8..f93abfae 100644 --- a/src/history/sqlite_backed.rs +++ b/src/history/sqlite_backed.rs @@ -327,13 +327,20 @@ impl SqliteBackedHistory { }; if let Some(command_line) = &query.filter.command_line { // TODO: escape % - let command_line_like = match command_line { - CommandLineSearch::Exact(e) => e.to_string(), - CommandLineSearch::Prefix(prefix) => format!("{prefix}*"), - CommandLineSearch::Substring(cont) => format!("*{cont}*"), + match command_line { + CommandLineSearch::Exact(e) => { + wheres.push("command_line == :command_line"); + params.push((":command_line", Box::new(e))); + } + CommandLineSearch::Prefix(prefix) => { + wheres.push("instr(command_line, :command_line) == 1"); + params.push((":command_line", Box::new(prefix))); + } + CommandLineSearch::Substring(cont) => { + wheres.push("instr(command_line, :command_line) >= 1"); + params.push((":command_line", Box::new(cont))); + } }; - wheres.push("command_line glob :command_line"); - params.push((":command_line", Box::new(command_line_like))); } if let Some(str) = &query.filter.not_command_line { From 5a89342c114da32e7bf6925c71df23c80e18bd7d Mon Sep 17 00:00:00 2001 From: sholderbach Date: Tue, 26 Mar 2024 15:34:11 +0100 Subject: [PATCH 4/4] Remove outdated fixme --- src/history/sqlite_backed.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/history/sqlite_backed.rs b/src/history/sqlite_backed.rs index f93abfae..830f1a60 100644 --- a/src/history/sqlite_backed.rs +++ b/src/history/sqlite_backed.rs @@ -326,7 +326,6 @@ impl SqliteBackedHistory { None => "", }; if let Some(command_line) = &query.filter.command_line { - // TODO: escape % match command_line { CommandLineSearch::Exact(e) => { wheres.push("command_line == :command_line");