Skip to content

Commit

Permalink
Fix case-consistency searching sqlite history (#777)
Browse files Browse the repository at this point in the history
* 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 nushell/nushell#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
  • Loading branch information
sholderbach authored Mar 27, 2024
1 parent 21903cc commit b7209b6
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 7 deletions.
18 changes: 18 additions & 0 deletions src/history/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
20 changes: 13 additions & 7 deletions src/history/sqlite_backed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b7209b6

Please sign in to comment.