From b7209b68721f9d33cc9c3f38ba7e7359e4b7c717 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Wed, 27 Mar 2024 22:04:03 +0100 Subject: [PATCH] Fix case-consistency searching sqlite history (#777) * 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. * Add test for case-sensitive prefix search Link the relevant issue so feature fans don't reintroduce bugs * Use sqlite `instr` function for case-exact match * Remove outdated fixme --- src/history/base.rs | 18 ++++++++++++++++++ src/history/sqlite_backed.rs | 20 +++++++++++++------- 2 files changed, 31 insertions(+), 7 deletions(-) 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()?; diff --git a/src/history/sqlite_backed.rs b/src/history/sqlite_backed.rs index ec3b021a..830f1a60 100644 --- a/src/history/sqlite_backed.rs +++ b/src/history/sqlite_backed.rs @@ -326,14 +326,20 @@ impl SqliteBackedHistory { None => "", }; 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 like :command_line"); - params.push((":command_line", Box::new(command_line_like))); } if let Some(str) = &query.filter.not_command_line {