From 8552ba15dc211c145fd48322de82e96f71678647 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 26 Jul 2022 14:48:18 +0200 Subject: [PATCH] Show symbols located in visible paths before ones located externally --- crates/collab/src/integration_tests.rs | 2 +- crates/fuzzy/src/fuzzy.rs | 2 +- crates/project/src/project.rs | 55 ++++----- crates/project_symbols/src/project_symbols.rs | 106 +++++++++++------- crates/workspace/src/workspace.rs | 12 +- 5 files changed, 103 insertions(+), 74 deletions(-) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index a3dcd5fbce..9471967eee 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -2589,7 +2589,7 @@ async fn test_project_symbols(cx_a: &mut TestAppContext, cx_b: &mut TestAppConte // Attempt to craft a symbol and violate host's privacy by opening an arbitrary file. let mut fake_symbol = symbols[0].clone(); - fake_symbol.path = Path::new("/code/secrets").into(); + fake_symbol.path.path = Path::new("/code/secrets").into(); let error = project_b .update(cx_b, |project, cx| { project.open_buffer_for_symbol(&fake_symbol, cx) diff --git a/crates/fuzzy/src/fuzzy.rs b/crates/fuzzy/src/fuzzy.rs index f6abb22ddc..401ab33d7f 100644 --- a/crates/fuzzy/src/fuzzy.rs +++ b/crates/fuzzy/src/fuzzy.rs @@ -181,7 +181,7 @@ pub async fn match_strings( cancel_flag: &AtomicBool, background: Arc, ) -> Vec { - if candidates.is_empty() { + if candidates.is_empty() || max_results == 0 { return Default::default(); } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e116761eb9..29106b5889 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -254,10 +254,9 @@ pub struct DocumentHighlight { #[derive(Clone, Debug)] pub struct Symbol { - pub source_worktree_id: WorktreeId, - pub worktree_id: WorktreeId, pub language_server_name: LanguageServerName, - pub path: PathBuf, + pub source_worktree_id: WorktreeId, + pub path: ProjectPath, pub label: CodeLabel, pub name: String, pub kind: lsp::SymbolKind, @@ -3324,16 +3323,19 @@ impl Project { if let Some((worktree, rel_path)) = this.find_local_worktree(&abs_path, cx) { - worktree_id = worktree.read(cx).id(); + worktree_id = (&worktree.read(cx)).id(); path = rel_path; } else { path = relativize_path(&worktree_abs_path, &abs_path); } - let signature = this.symbol_signature(worktree_id, &path); - let language = this.languages.select_language(&path); + let project_path = ProjectPath { + worktree_id, + path: path.into(), + }; + let signature = this.symbol_signature(&project_path); + let language = this.languages.select_language(&project_path.path); let language_server_name = adapter.name.clone(); - Some(async move { let label = if let Some(language) = language { language @@ -3344,15 +3346,14 @@ impl Project { }; Symbol { - source_worktree_id, - worktree_id, language_server_name, + source_worktree_id, + path: project_path, label: label.unwrap_or_else(|| { CodeLabel::plain(lsp_symbol.name.clone(), None) }), kind: lsp_symbol.kind, name: lsp_symbol.name, - path, range: range_from_lsp(lsp_symbol.location.range), signature, } @@ -3410,7 +3411,7 @@ impl Project { }; let worktree_abs_path = if let Some(worktree_abs_path) = self - .worktree_for_id(symbol.worktree_id, cx) + .worktree_for_id(symbol.path.worktree_id, cx) .and_then(|worktree| worktree.read(cx).as_local()) .map(|local_worktree| local_worktree.abs_path()) { @@ -3418,7 +3419,7 @@ impl Project { } else { return Task::ready(Err(anyhow!("worktree not found for symbol"))); }; - let symbol_abs_path = worktree_abs_path.join(&symbol.path); + let symbol_abs_path = worktree_abs_path.join(&symbol.path.path); let symbol_uri = if let Ok(uri) = lsp::Url::from_file_path(symbol_abs_path) { uri } else { @@ -4622,11 +4623,11 @@ impl Project { self.active_entry } - pub fn entry_for_path(&self, path: &ProjectPath, cx: &AppContext) -> Option { + pub fn entry_for_path(&self, path: &ProjectPath, cx: &AppContext) -> Option { self.worktree_for_id(path.worktree_id, cx)? .read(cx) .entry_for_path(&path.path) - .map(|entry| entry.id) + .cloned() } pub fn path_for_entry(&self, entry_id: ProjectEntryId, cx: &AppContext) -> Option { @@ -5436,7 +5437,7 @@ impl Project { .read_with(&cx, |this, _| this.deserialize_symbol(symbol)) .await?; let symbol = this.read_with(&cx, |this, _| { - let signature = this.symbol_signature(symbol.worktree_id, &symbol.path); + let signature = this.symbol_signature(&symbol.path); if signature == symbol.signature { Ok(symbol) } else { @@ -5454,10 +5455,10 @@ impl Project { }) } - fn symbol_signature(&self, worktree_id: WorktreeId, path: &Path) -> [u8; 32] { + fn symbol_signature(&self, project_path: &ProjectPath) -> [u8; 32] { let mut hasher = Sha256::new(); - hasher.update(worktree_id.to_proto().to_be_bytes()); - hasher.update(path.to_string_lossy().as_bytes()); + hasher.update(project_path.worktree_id.to_proto().to_be_bytes()); + hasher.update(project_path.path.to_string_lossy().as_bytes()); hasher.update(self.nonce.to_be_bytes()); hasher.finalize().as_slice().try_into().unwrap() } @@ -5655,14 +5656,17 @@ impl Project { .end .ok_or_else(|| anyhow!("invalid end"))?; let kind = unsafe { mem::transmute(serialized_symbol.kind) }; - let path = PathBuf::from(serialized_symbol.path); - let language = languages.select_language(&path); - Ok(Symbol { - source_worktree_id, + let path = ProjectPath { worktree_id, + path: PathBuf::from(serialized_symbol.path).into(), + }; + let language = languages.select_language(&path.path); + Ok(Symbol { language_server_name: LanguageServerName( serialized_symbol.language_server_name.into(), ), + source_worktree_id, + path, label: { match language { Some(language) => { @@ -5676,7 +5680,6 @@ impl Project { }, name: serialized_symbol.name, - path, range: PointUtf16::new(start.row, start.column) ..PointUtf16::new(end.row, end.column), kind, @@ -6142,12 +6145,12 @@ impl From for fs::RemoveOptions { fn serialize_symbol(symbol: &Symbol) -> proto::Symbol { proto::Symbol { - source_worktree_id: symbol.source_worktree_id.to_proto(), - worktree_id: symbol.worktree_id.to_proto(), language_server_name: symbol.language_server_name.0.to_string(), + source_worktree_id: symbol.source_worktree_id.to_proto(), + worktree_id: symbol.path.worktree_id.to_proto(), + path: symbol.path.path.to_string_lossy().to_string(), name: symbol.name.clone(), kind: unsafe { mem::transmute(symbol.kind) }, - path: symbol.path.to_string_lossy().to_string(), start: Some(proto::Point { row: symbol.range.start.row, column: symbol.range.start.column, diff --git a/crates/project_symbols/src/project_symbols.rs b/crates/project_symbols/src/project_symbols.rs index 10425f63a8..8f2305eaff 100644 --- a/crates/project_symbols/src/project_symbols.rs +++ b/crates/project_symbols/src/project_symbols.rs @@ -26,7 +26,8 @@ pub struct ProjectSymbolsView { project: ModelHandle, selected_match_index: usize, symbols: Vec, - match_candidates: Vec, + visible_match_candidates: Vec, + external_match_candidates: Vec, show_worktree_root_name: bool, pending_update: Task<()>, matches: Vec, @@ -63,7 +64,8 @@ impl ProjectSymbolsView { picker: cx.add_view(|cx| Picker::new(handle, cx)), selected_match_index: 0, symbols: Default::default(), - match_candidates: Default::default(), + visible_match_candidates: Default::default(), + external_match_candidates: Default::default(), matches: Default::default(), show_worktree_root_name: false, pending_update: Task::ready(()), @@ -80,38 +82,39 @@ impl ProjectSymbolsView { } fn filter(&mut self, query: &str, cx: &mut ViewContext) { - let mut matches = if query.is_empty() { - self.match_candidates - .iter() - .enumerate() - .map(|(candidate_id, candidate)| StringMatch { - candidate_id, - score: Default::default(), - positions: Default::default(), - string: candidate.string.clone(), - }) - .collect() - } else { - cx.background_executor().block(fuzzy::match_strings( - &self.match_candidates, - query, - false, - 100, - &Default::default(), - cx.background().clone(), - )) - }; - - matches.sort_unstable_by_key(|mat| { - let label = &self.symbols[mat.candidate_id].label; + const MAX_MATCHES: usize = 100; + let mut visible_matches = cx.background_executor().block(fuzzy::match_strings( + &self.visible_match_candidates, + query, + false, + MAX_MATCHES, + &Default::default(), + cx.background().clone(), + )); + let mut external_matches = cx.background_executor().block(fuzzy::match_strings( + &self.external_match_candidates, + query, + false, + MAX_MATCHES - visible_matches.len(), + &Default::default(), + cx.background().clone(), + )); + let sort_key_for_match = |mat: &StringMatch| { + let symbol = &self.symbols[mat.candidate_id]; ( Reverse(OrderedFloat(mat.score)), - &label.text[label.filter_range.clone()], + &symbol.label.text[symbol.label.filter_range.clone()], ) - }); + }; + + visible_matches.sort_unstable_by_key(sort_key_for_match); + external_matches.sort_unstable_by_key(sort_key_for_match); + let mut matches = visible_matches; + matches.append(&mut external_matches); for mat in &mut matches { - let filter_start = self.symbols[mat.candidate_id].label.filter_range.start; + let symbol = &self.symbols[mat.candidate_id]; + let filter_start = symbol.label.filter_range.start; for position in &mut mat.positions { *position += filter_start; } @@ -198,7 +201,8 @@ impl PickerDelegate for ProjectSymbolsView { if let Some(this) = this.upgrade(&cx) { if let Some(symbols) = symbols { this.update(&mut cx, |this, cx| { - this.match_candidates = symbols + let project = this.project.read(cx); + let (visible_match_candidates, external_match_candidates) = symbols .iter() .enumerate() .map(|(id, symbol)| { @@ -208,7 +212,14 @@ impl PickerDelegate for ProjectSymbolsView { .to_string(), ) }) - .collect(); + .partition(|candidate| { + project + .entry_for_path(&symbols[candidate.id].path, cx) + .map_or(false, |e| !e.is_ignored) + }); + + this.visible_match_candidates = visible_match_candidates; + this.external_match_candidates = external_match_candidates; this.symbols = symbols; this.filter(&query, cx); }); @@ -232,10 +243,10 @@ impl PickerDelegate for ProjectSymbolsView { let symbol = &self.symbols[string_match.candidate_id]; let syntax_runs = styled_runs_for_code_label(&symbol.label, &settings.theme.editor.syntax); - let mut path = symbol.path.to_string_lossy(); + let mut path = symbol.path.path.to_string_lossy(); if self.show_worktree_root_name { let project = self.project.read(cx); - if let Some(worktree) = project.worktree_for_id(symbol.worktree_id, cx) { + if let Some(worktree) = project.worktree_for_id(symbol.path.worktree_id, cx) { path = Cow::Owned(format!( "{}{}{}", worktree.read(cx).root_name(), @@ -275,7 +286,7 @@ mod tests { use gpui::{serde_json::json, TestAppContext}; use language::{FakeLspAdapter, Language, LanguageConfig}; use project::FakeFs; - use std::sync::Arc; + use std::{path::Path, sync::Arc}; #[gpui::test] async fn test_project_symbols(cx: &mut TestAppContext) { @@ -309,15 +320,21 @@ mod tests { // Set up fake langauge server to return fuzzy matches against // a fixed set of symbol names. - let fake_symbol_names = ["one", "ton", "uno"]; + let fake_symbols = [ + symbol("one", "/external"), + symbol("ton", "/dir/test.rs"), + symbol("uno", "/dir/test.rs"), + ]; let fake_server = fake_servers.next().await.unwrap(); fake_server.handle_request::( move |params: lsp::WorkspaceSymbolParams, cx| { let executor = cx.background(); + let fake_symbols = fake_symbols.clone(); async move { - let candidates = fake_symbol_names - .into_iter() - .map(|name| StringMatchCandidate::new(0, name.into())) + let candidates = fake_symbols + .iter() + .enumerate() + .map(|(id, symbol)| StringMatchCandidate::new(id, symbol.name.clone())) .collect::>(); let matches = if params.query.is_empty() { Vec::new() @@ -334,7 +351,10 @@ mod tests { }; Ok(Some( - matches.into_iter().map(|mat| symbol(&mat.string)).collect(), + matches + .into_iter() + .map(|mat| fake_symbols[mat.candidate_id].clone()) + .collect(), )) } }, @@ -367,8 +387,8 @@ mod tests { cx.foreground().run_until_parked(); symbols_view.read_with(cx, |symbols_view, _| { assert_eq!(symbols_view.matches.len(), 2); - assert_eq!(symbols_view.matches[0].string, "one"); - assert_eq!(symbols_view.matches[1].string, "ton"); + assert_eq!(symbols_view.matches[0].string, "ton"); + assert_eq!(symbols_view.matches[1].string, "one"); }); // Spawn more updates such that in the end, there are again no matches. @@ -383,7 +403,7 @@ mod tests { }); } - fn symbol(name: &str) -> lsp::SymbolInformation { + fn symbol(name: &str, path: impl AsRef) -> lsp::SymbolInformation { #[allow(deprecated)] lsp::SymbolInformation { name: name.to_string(), @@ -392,7 +412,7 @@ mod tests { deprecated: None, container_name: None, location: lsp::Location::new( - lsp::Url::from_file_path("/a/b").unwrap(), + lsp::Url::from_file_path(path.as_ref()).unwrap(), lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)), ), } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 0321e770cb..e8ff1932d4 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2821,7 +2821,9 @@ mod tests { project.read_with(cx, |project, cx| { assert_eq!( project.active_entry(), - project.entry_for_path(&(worktree_id, "one.txt").into(), cx) + project + .entry_for_path(&(worktree_id, "one.txt").into(), cx) + .map(|e| e.id) ); }); assert_eq!( @@ -2838,7 +2840,9 @@ mod tests { project.read_with(cx, |project, cx| { assert_eq!( project.active_entry(), - project.entry_for_path(&(worktree_id, "two.txt").into(), cx) + project + .entry_for_path(&(worktree_id, "two.txt").into(), cx) + .map(|e| e.id) ); }); @@ -2856,7 +2860,9 @@ mod tests { project.read_with(cx, |project, cx| { assert_eq!( project.active_entry(), - project.entry_for_path(&(worktree_id, "one.txt").into(), cx) + project + .entry_for_path(&(worktree_id, "one.txt").into(), cx) + .map(|e| e.id) ); });