Show symbols located in visible paths before ones located externally

Antonio Scandurra created

Change summary

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(-)

Detailed changes

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)

crates/fuzzy/src/fuzzy.rs 🔗

@@ -181,7 +181,7 @@ pub async fn match_strings(
     cancel_flag: &AtomicBool,
     background: Arc<executor::Background>,
 ) -> Vec<StringMatch> {
-    if candidates.is_empty() {
+    if candidates.is_empty() || max_results == 0 {
         return Default::default();
     }
 

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<ProjectEntryId> {
+    pub fn entry_for_path(&self, path: &ProjectPath, cx: &AppContext) -> Option<Entry> {
         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<ProjectPath> {
@@ -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<lsp::DeleteFileOptions> 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,

crates/project_symbols/src/project_symbols.rs 🔗

@@ -26,7 +26,8 @@ pub struct ProjectSymbolsView {
     project: ModelHandle<Project>,
     selected_match_index: usize,
     symbols: Vec<Symbol>,
-    match_candidates: Vec<StringMatchCandidate>,
+    visible_match_candidates: Vec<StringMatchCandidate>,
+    external_match_candidates: Vec<StringMatchCandidate>,
     show_worktree_root_name: bool,
     pending_update: Task<()>,
     matches: Vec<StringMatch>,
@@ -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<Self>) {
-        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::<lsp::request::WorkspaceSymbol, _, _>(
             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::<Vec<_>>();
                     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<Path>) -> 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)),
             ),
         }

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)
             );
         });