Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

project search: Persist search history across session (#9932)

Browse source 
Partially implements #9717, persistence between restarts is currently
missing, but I would like to get feedback on the implementation first.

Previously the search history was not saved across different project
searches. As the `SearchHistory` is now maintained inside of the
project, it can be persisted across different project searches.

I also removed the behavior that a new query replaces the previous
search query, if it contains the text of the previous query.
I believe this was only intended to make buffer search work, therefore I
disabled this behavior but only for the project search.

Currently when you navigated through the queries the tab title changed
even if the search was not started, which doesn't make sense to me.
Current behavior:


https://github.com/zed-industries/zed/assets/53836821/1c365702-e93c-4cab-a1eb-0af3fef95476


With this PR the tab header will actually keep the search name until you
start another search again.

---

Showcase:


https://github.com/zed-industries/zed/assets/53836821/c0d6e496-915f-44bc-be16-12d7c3cda2d7


Release Notes:

- Added support for persisting project search history across a session
- Fixed tab header of project search changing when cycling through
search history, even when there is no search submitted
This commit is contained in:
Bennet Bo Fenner 2024-04-02 11:13:18 +02:00 • committed by GitHub
parent c15b9d4e1c
commit 1dbd520cc9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 566 additions and 216 deletions
Download patch file Download diff file Expand all files Collapse all files

1
crates/search/Cargo.toml
View file

@ -25,7 +25,6 @@ project.workspace = true
serde.workspace = true
serde_json.workspace = true
settings.workspace = true
smallvec.workspace = true
smol.workspace = true
theme.workspace = true
ui.workspace = true

38
crates/search/src/buffer_search.rs
View file

@ -1,7 +1,6 @@
mod registrar;
use crate::{
history::SearchHistory,
mode::{next_mode, SearchMode},
search_bar::render_nav_button,
ActivateRegexMode, ActivateTextMode, CycleMode, NextHistoryQuery, PreviousHistoryQuery,
@ -20,7 +19,10 @@ use gpui::{
ParentElement as _, Render, Styled, Subscription, Task, TextStyle, View, ViewContext,
VisualContext as _, WhiteSpace, WindowContext,
};
use project::search::SearchQuery;
use project::{
search::SearchQuery,
search_history::{SearchHistory, SearchHistoryCursor},
};
use serde::Deserialize;
use settings::Settings;
use std::{any::Any, sync::Arc};
@ -39,6 +41,7 @@ use registrar::{ForDeployed, ForDismissed, SearchActionsRegistrar, WithResults};
const MIN_INPUT_WIDTH_REMS: f32 = 15.;
const MAX_INPUT_WIDTH_REMS: f32 = 30.;
const MAX_BUFFER_SEARCH_HISTORY_SIZE: usize = 50;
#[derive(PartialEq, Clone, Deserialize)]
pub struct Deploy {
@ -75,6 +78,7 @@ pub struct BufferSearchBar {
query_contains_error: bool,
dismissed: bool,
search_history: SearchHistory,
search_history_cursor: SearchHistoryCursor,
current_mode: SearchMode,
replace_enabled: bool,
}
@ -526,6 +530,7 @@ impl BufferSearchBar {
let replacement_editor = cx.new_view(|cx| Editor::single_line(cx));
cx.subscribe(&replacement_editor, Self::on_replacement_editor_event)
.detach();
Self {
query_editor,
query_editor_focused: false,
@ -540,7 +545,11 @@ impl BufferSearchBar {
pending_search: None,
query_contains_error: false,
dismissed: true,
search_history: SearchHistory::default(),
search_history: SearchHistory::new(
Some(MAX_BUFFER_SEARCH_HISTORY_SIZE),
project::search_history::QueryInsertionBehavior::ReplacePreviousIfContains,
),
search_history_cursor: Default::default(),
current_mode: SearchMode::default(),
active_search: None,
replace_enabled: false,
@ -934,7 +943,8 @@ impl BufferSearchBar {
.insert(active_searchable_item.downgrade(), matches);
this.update_match_index(cx);
this.search_history.add(query_text);
this.search_history
.add(&mut this.search_history_cursor, query_text);
if !this.dismissed {
let matches = this
.searchable_items_with_matches
@ -996,23 +1006,35 @@ impl BufferSearchBar {
}
fn next_history_query(&mut self, _: &NextHistoryQuery, cx: &mut ViewContext<Self>) {
if let Some(new_query) = self.search_history.next().map(str::to_string) {
if let Some(new_query) = self
.search_history
.next(&mut self.search_history_cursor)
.map(str::to_string)
{
let _ = self.search(&new_query, Some(self.search_options), cx);
} else {
self.search_history.reset_selection();
self.search_history_cursor.reset();
let _ = self.search("", Some(self.search_options), cx);
}
}
fn previous_history_query(&mut self, _: &PreviousHistoryQuery, cx: &mut ViewContext<Self>) {
if self.query(cx).is_empty() {
if let Some(new_query) = self.search_history.current().map(str::to_string) {
if let Some(new_query) = self
.search_history
.current(&mut self.search_history_cursor)
.map(str::to_string)
{
let _ = self.search(&new_query, Some(self.search_options), cx);
return;
}
}
if let Some(new_query) = self.search_history.previous().map(str::to_string) {
if let Some(new_query) = self
.search_history
.previous(&mut self.search_history_cursor)
.map(str::to_string)
{
let _ = self.search(&new_query, Some(self.search_options), cx);
}
}

184
crates/search/src/history.rs
View file

@ -1,184 +0,0 @@
use smallvec::SmallVec;
const SEARCH_HISTORY_LIMIT: usize = 20;
#[derive(Default, Debug, Clone)]
pub struct SearchHistory {
history: SmallVec<[String; SEARCH_HISTORY_LIMIT]>,
selected: Option<usize>,
}
impl SearchHistory {
pub fn add(&mut self, search_string: String) {
if let Some(i) = self.selected {
if search_string == self.history[i] {
return;
}
}
if let Some(previously_searched) = self.history.last_mut() {
if search_string.contains(previously_searched.as_str()) {
*previously_searched = search_string;
self.selected = Some(self.history.len() - 1);
return;
}
}
self.history.push(search_string);
if self.history.len() > SEARCH_HISTORY_LIMIT {
self.history.remove(0);
}
self.selected = Some(self.history.len() - 1);
}
pub fn next(&mut self) -> Option<&str> {
let history_size = self.history.len();
if history_size == 0 {
return None;
}
let selected = self.selected?;
if selected == history_size - 1 {
return None;
}
let next_index = selected + 1;
self.selected = Some(next_index);
Some(&self.history[next_index])
}
pub fn current(&self) -> Option<&str> {
Some(&self.history[self.selected?])
}
pub fn previous(&mut self) -> Option<&str> {
let history_size = self.history.len();
if history_size == 0 {
return None;
}
let prev_index = match self.selected {
Some(selected_index) => {
if selected_index == 0 {
return None;
} else {
selected_index - 1
}
}
None => history_size - 1,
};
self.selected = Some(prev_index);
Some(&self.history[prev_index])
}
pub fn reset_selection(&mut self) {
self.selected = None;
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_add() {
let mut search_history = SearchHistory::default();
assert_eq!(
search_history.current(),
None,
"No current selection should be set for the default search history"
);
search_history.add("rust".to_string());
assert_eq!(
search_history.current(),
Some("rust"),
"Newly added item should be selected"
);
// check if duplicates are not added
search_history.add("rust".to_string());
assert_eq!(
search_history.history.len(),
1,
"Should not add a duplicate"
);
assert_eq!(search_history.current(), Some("rust"));
// check if new string containing the previous string replaces it
search_history.add("rustlang".to_string());
assert_eq!(
search_history.history.len(),
1,
"Should replace previous item if it's a substring"
);
assert_eq!(search_history.current(), Some("rustlang"));
// push enough items to test SEARCH_HISTORY_LIMIT
for i in 0..SEARCH_HISTORY_LIMIT * 2 {
search_history.add(format!("item{i}"));
}
assert!(search_history.history.len() <= SEARCH_HISTORY_LIMIT);
}
#[test]
fn test_next_and_previous() {
let mut search_history = SearchHistory::default();
assert_eq!(
search_history.next(),
None,
"Default search history should not have a next item"
);
search_history.add("Rust".to_string());
assert_eq!(search_history.next(), None);
search_history.add("JavaScript".to_string());
assert_eq!(search_history.next(), None);
search_history.add("TypeScript".to_string());
assert_eq!(search_history.next(), None);
assert_eq!(search_history.current(), Some("TypeScript"));
assert_eq!(search_history.previous(), Some("JavaScript"));
assert_eq!(search_history.current(), Some("JavaScript"));
assert_eq!(search_history.previous(), Some("Rust"));
assert_eq!(search_history.current(), Some("Rust"));
assert_eq!(search_history.previous(), None);
assert_eq!(search_history.current(), Some("Rust"));
assert_eq!(search_history.next(), Some("JavaScript"));
assert_eq!(search_history.current(), Some("JavaScript"));
assert_eq!(search_history.next(), Some("TypeScript"));
assert_eq!(search_history.current(), Some("TypeScript"));
assert_eq!(search_history.next(), None);
assert_eq!(search_history.current(), Some("TypeScript"));
}
#[test]
fn test_reset_selection() {
let mut search_history = SearchHistory::default();
search_history.add("Rust".to_string());
search_history.add("JavaScript".to_string());
search_history.add("TypeScript".to_string());
assert_eq!(search_history.current(), Some("TypeScript"));
search_history.reset_selection();
assert_eq!(search_history.current(), None);
assert_eq!(
search_history.previous(),
Some("TypeScript"),
"Should start from the end after reset on previous item query"
);
search_history.previous();
assert_eq!(search_history.current(), Some("JavaScript"));
search_history.previous();
assert_eq!(search_history.current(), Some("Rust"));
search_history.reset_selection();
assert_eq!(search_history.current(), None);
}
}

274
crates/search/src/project_search.rs
View file

@ -1,8 +1,7 @@
use crate::{
history::SearchHistory, mode::SearchMode, ActivateRegexMode, ActivateTextMode, CycleMode,
NextHistoryQuery, PreviousHistoryQuery, ReplaceAll, ReplaceNext, SearchOptions,
SelectNextMatch, SelectPrevMatch, ToggleCaseSensitive, ToggleIncludeIgnored, ToggleReplace,
ToggleWholeWord,
mode::SearchMode, ActivateRegexMode, ActivateTextMode, CycleMode, NextHistoryQuery,
PreviousHistoryQuery, ReplaceAll, ReplaceNext, SearchOptions, SelectNextMatch, SelectPrevMatch,
ToggleCaseSensitive, ToggleIncludeIgnored, ToggleReplace, ToggleWholeWord,
};
use anyhow::Context as _;
use collections::{HashMap, HashSet};
@ -20,7 +19,7 @@ use gpui::{
WeakModel, WeakView, WhiteSpace, WindowContext,
};
use menu::Confirm;
use project::{search::SearchQuery, Project};
use project::{search::SearchQuery, search_history::SearchHistoryCursor, Project};
use settings::Settings;
use smol::stream::StreamExt;
use std::{
@ -125,10 +124,11 @@ struct ProjectSearch {
pending_search: Option<Task<Option<()>>>,
match_ranges: Vec<Range<Anchor>>,
active_query: Option<SearchQuery>,
last_search_query_text: Option<String>,
search_id: usize,
search_history: SearchHistory,
no_results: Option<bool>,
limit_reached: bool,
search_history_cursor: SearchHistoryCursor,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -180,10 +180,11 @@ impl ProjectSearch {
pending_search: Default::default(),
match_ranges: Default::default(),
active_query: None,
last_search_query_text: None,
search_id: 0,
search_history: SearchHistory::default(),
no_results: None,
limit_reached: false,
search_history_cursor: Default::default(),
}
}
@ -196,19 +197,23 @@ impl ProjectSearch {
pending_search: Default::default(),
match_ranges: self.match_ranges.clone(),
active_query: self.active_query.clone(),
last_search_query_text: self.last_search_query_text.clone(),
search_id: self.search_id,
search_history: self.search_history.clone(),
no_results: self.no_results,
limit_reached: self.limit_reached,
search_history_cursor: self.search_history_cursor.clone(),
})
}
fn search(&mut self, query: SearchQuery, cx: &mut ModelContext<Self>) {
let search = self
.project
.update(cx, |project, cx| project.search(query.clone(), cx));
let search = self.project.update(cx, |project, cx| {
project
.search_history_mut()
.add(&mut self.search_history_cursor, query.as_str().to_string());
project.search(query.clone(), cx)
});
self.last_search_query_text = Some(query.as_str().to_string());
self.search_id += 1;
self.search_history.add(query.as_str().to_string());
self.active_query = Some(query);
self.match_ranges.clear();
self.pending_search = Some(cx.spawn(|this, mut cx| async move {
@ -368,8 +373,7 @@ impl Item for ProjectSearchView {
let last_query: Option<SharedString> = self
.model
.read(cx)
.search_history
.current()
.last_search_query_text
.as_ref()
.map(|query| {
let query = query.replace('\n', "");
@ -1270,11 +1274,16 @@ impl ProjectSearchBar {
fn next_history_query(&mut self, _: &NextHistoryQuery, cx: &mut ViewContext<Self>) {
if let Some(search_view) = self.active_project_search.as_ref() {
search_view.update(cx, |search_view, cx| {
let new_query = search_view.model.update(cx, |model, _| {
if let Some(new_query) = model.search_history.next().map(str::to_string) {
let new_query = search_view.model.update(cx, |model, cx| {
if let Some(new_query) = model.project.update(cx, |project, _| {
project
.search_history_mut()
.next(&mut model.search_history_cursor)
.map(str::to_string)
}) {
new_query
} else {
model.search_history.reset_selection();
model.search_history_cursor.reset();
String::new()
}
});
@ -1290,8 +1299,10 @@ impl ProjectSearchBar {
if let Some(new_query) = search_view
.model
.read(cx)
.search_history
.current()
.project
.read(cx)
.search_history()
.current(&search_view.model.read(cx).search_history_cursor)
.map(str::to_string)
{
search_view.set_query(&new_query, cx);
@ -1299,8 +1310,13 @@ impl ProjectSearchBar {
}
}
if let Some(new_query) = search_view.model.update(cx, |model, _| {
model.search_history.previous().map(str::to_string)
if let Some(new_query) = search_view.model.update(cx, |model, cx| {
model.project.update(cx, |project, _| {
project
.search_history_mut()
.previous(&mut model.search_history_cursor)
.map(str::to_string)
})
}) {
search_view.set_query(&new_query, cx);
}
@ -2904,6 +2920,222 @@ pub mod tests {
.unwrap();
}
#[gpui::test]
async fn test_search_query_history_with_multiple_views(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.background_executor.clone());
fs.insert_tree(
"/dir",
json!({
"one.rs": "const ONE: usize = 1;",
}),
)
.await;
let project = Project::test(fs.clone(), ["/dir".as_ref()], cx).await;
let worktree_id = project.update(cx, |this, cx| {
this.worktrees().next().unwrap().read(cx).id()
});
let window = cx.add_window(|cx| Workspace::test_new(project, cx));
let workspace = window.root(cx).unwrap();
let panes: Vec<_> = window
.update(cx, |this, _| this.panes().to_owned())
.unwrap();
let search_bar_1 = window.build_view(cx, |_| ProjectSearchBar::new());
let search_bar_2 = window.build_view(cx, |_| ProjectSearchBar::new());
assert_eq!(panes.len(), 1);
let first_pane = panes.get(0).cloned().unwrap();
assert_eq!(cx.update(|cx| first_pane.read(cx).items_len()), 0);
window
.update(cx, |workspace, cx| {
workspace.open_path(
(worktree_id, "one.rs"),
Some(first_pane.downgrade()),
true,
cx,
)
})
.unwrap()
.await
.unwrap();
assert_eq!(cx.update(|cx| first_pane.read(cx).items_len()), 1);
// Add a project search item to the first pane
window
.update(cx, {
let search_bar = search_bar_1.clone();
let pane = first_pane.clone();
move |workspace, cx| {
pane.update(cx, move |pane, cx| {
pane.toolbar()
.update(cx, |toolbar, cx| toolbar.add_item(search_bar, cx))
});
ProjectSearchView::new_search(workspace, &workspace::NewSearch, cx)
}
})
.unwrap();
let search_view_1 = cx.read(|cx| {
workspace
.read(cx)
.active_item(cx)
.and_then(|item| item.downcast::<ProjectSearchView>())
.expect("Search view expected to appear after new search event trigger")
});
let second_pane = window
.update(cx, |workspace, cx| {
workspace.split_and_clone(first_pane.clone(), workspace::SplitDirection::Right, cx)
})
.unwrap()
.unwrap();
assert_eq!(cx.update(|cx| second_pane.read(cx).items_len()), 1);
assert_eq!(cx.update(|cx| second_pane.read(cx).items_len()), 1);
assert_eq!(cx.update(|cx| first_pane.read(cx).items_len()), 2);
// Add a project search item to the second pane
window
.update(cx, {
let search_bar = search_bar_2.clone();
let pane = second_pane.clone();
move |workspace, cx| {
assert_eq!(workspace.panes().len(), 2);
pane.update(cx, move |pane, cx| {
pane.toolbar()
.update(cx, |toolbar, cx| toolbar.add_item(search_bar, cx))
});
ProjectSearchView::new_search(workspace, &workspace::NewSearch, cx)
}
})
.unwrap();
let search_view_2 = cx.read(|cx| {
workspace
.read(cx)
.active_item(cx)
.and_then(|item| item.downcast::<ProjectSearchView>())
.expect("Search view expected to appear after new search event trigger")
});
cx.run_until_parked();
assert_eq!(cx.update(|cx| first_pane.read(cx).items_len()), 2);
assert_eq!(cx.update(|cx| second_pane.read(cx).items_len()), 2);
let update_search_view =
|search_view: &View<ProjectSearchView>, query: &str, cx: &mut TestAppContext| {
window
.update(cx, |_, cx| {
search_view.update(cx, |search_view, cx| {
search_view
.query_editor
.update(cx, |query_editor, cx| query_editor.set_text(query, cx));
search_view.search(cx);
});
})
.unwrap();
};
let active_query =
|search_view: &View<ProjectSearchView>, cx: &mut TestAppContext| -> String {
window
.update(cx, |_, cx| {
search_view.update(cx, |search_view, cx| {
search_view.query_editor.read(cx).text(cx).to_string()
})
})
.unwrap()
};
let select_prev_history_item =
|search_bar: &View<ProjectSearchBar>, cx: &mut TestAppContext| {
window
.update(cx, |_, cx| {
search_bar.update(cx, |search_bar, cx| {
search_bar.previous_history_query(&PreviousHistoryQuery, cx);
})
})
.unwrap();
};
let select_next_history_item =
|search_bar: &View<ProjectSearchBar>, cx: &mut TestAppContext| {
window
.update(cx, |_, cx| {
search_bar.update(cx, |search_bar, cx| {
search_bar.next_history_query(&NextHistoryQuery, cx);
})
})
.unwrap();
};
update_search_view(&search_view_1, "ONE", cx);
cx.background_executor.run_until_parked();
update_search_view(&search_view_2, "TWO", cx);
cx.background_executor.run_until_parked();
assert_eq!(active_query(&search_view_1, cx), "ONE");
assert_eq!(active_query(&search_view_2, cx), "TWO");
// Selecting previous history item should select the query from search view 1.
select_prev_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "ONE");
// Selecting the previous history item should not change the query as it is already the first item.
select_prev_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "ONE");
// Changing the query in search view 2 should not affect the history of search view 1.
assert_eq!(active_query(&search_view_1, cx), "ONE");
// Deploying a new search in search view 2
update_search_view(&search_view_2, "THREE", cx);
cx.background_executor.run_until_parked();
select_next_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "");
select_prev_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "THREE");
select_prev_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "TWO");
select_prev_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "ONE");
select_prev_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "ONE");
// Search view 1 should now see the query from search view 2.
assert_eq!(active_query(&search_view_1, cx), "ONE");
select_next_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "TWO");
// Here is the new query from search view 2
select_next_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "THREE");
select_next_history_item(&search_bar_2, cx);
assert_eq!(active_query(&search_view_2, cx), "");
select_next_history_item(&search_bar_1, cx);
assert_eq!(active_query(&search_view_1, cx), "TWO");
select_next_history_item(&search_bar_1, cx);
assert_eq!(active_query(&search_view_1, cx), "THREE");
select_next_history_item(&search_bar_1, cx);
assert_eq!(active_query(&search_view_1, cx), "");
}
#[gpui::test]
async fn test_deploy_search_with_multiple_panes(cx: &mut TestAppContext) {
init_test(cx);

1
crates/search/src/search.rs
View file

@ -8,7 +8,6 @@ use ui::{prelude::*, Tooltip};
use ui::{ButtonStyle, IconButton};
pub mod buffer_search;
mod history;
mod mode;
pub mod project_search;
pub(crate) mod search_bar;