Sort completions by relevance for strong matches (#20145)

Avinash Thakur created 1 year ago

Further enhancement:
On exploring VSCode's sorting logic, there are two major distinctions:
* A config option exists to adjust sort priority of snippets. They can
be placed inline (default), top or at bottom of completitions.
* The sorting order sorts by (in order): sort_text (lower case),
sort_text, kind

ref:
https://github.com/microsoft/vscode/blob/6f2d4781e857a550b7e9b05d40b969c840e264db/src/vs/editor/contrib/suggest/browser/suggest.ts#L338-L383

Closes #19786

Release Notes:

- Improved sort order in completions to show relevant matches first
([#19786](https://github.com/zed-industries/zed/issues/19786))

Change summary

crates/editor/src/editor.rs       | 16 +++---
crates/editor/src/editor_tests.rs | 68 +++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 8 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -1353,22 +1353,22 @@ impl CompletionsMenu {
                 // Strong matches are the ones with a high fuzzy-matcher score (the "obvious" matches)
                 // and the Weak matches are the rest.
                 //
-                // For the strong matches, we sort by the language-servers score first and for the weak
-                // matches, we prefer our fuzzy finder first.
+                // For the strong matches, we sort by our fuzzy-finder score first and for the weak
+                // matches, we prefer language-server sort_text first.
                 //
-                // The thinking behind that: it's useless to take the sort_text the language-server gives
-                // us into account when it's obviously a bad match.
+                // The thinking behind that: we want to show strong matches first in order of relevance(fuzzy score).
+                // Rest of the matches(weak) can be sorted as language-server expects.
 
                 #[derive(PartialEq, Eq, PartialOrd, Ord)]
                 enum MatchScore<'a> {
                     Strong {
-                        sort_text: Option<&'a str>,
                         score: Reverse<OrderedFloat<f64>>,
+                        sort_text: Option<&'a str>,
                         sort_key: (usize, &'a str),
                     },
                     Weak {
-                        score: Reverse<OrderedFloat<f64>>,
                         sort_text: Option<&'a str>,
+                        score: Reverse<OrderedFloat<f64>>,
                         sort_key: (usize, &'a str),
                     },
                 }
@@ -1380,14 +1380,14 @@ impl CompletionsMenu {
 
                 if mat.score >= 0.2 {
                     MatchScore::Strong {
-                        sort_text,
                         score,
+                        sort_text,
                         sort_key,
                     }
                 } else {
                     MatchScore::Weak {
-                        score,
                         sort_text,
+                        score,
                         sort_key,
                     }
                 }

crates/editor/src/editor_tests.rs

@@ -8385,6 +8385,74 @@ async fn test_completion_page_up_down_keys(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_completion_sort(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            completion_provider: Some(lsp::CompletionOptions {
+                trigger_characters: Some(vec![".".to_string()]),
+                ..Default::default()
+            }),
+            ..Default::default()
+        },
+        cx,
+    )
+    .await;
+    cx.lsp
+        .handle_request::<lsp::request::Completion, _, _>(move |_, _| async move {
+            Ok(Some(lsp::CompletionResponse::Array(vec![
+                lsp::CompletionItem {
+                    label: "Range".into(),
+                    sort_text: Some("a".into()),
+                    ..Default::default()
+                },
+                lsp::CompletionItem {
+                    label: "r".into(),
+                    sort_text: Some("b".into()),
+                    ..Default::default()
+                },
+                lsp::CompletionItem {
+                    label: "ret".into(),
+                    sort_text: Some("c".into()),
+                    ..Default::default()
+                },
+                lsp::CompletionItem {
+                    label: "return".into(),
+                    sort_text: Some("d".into()),
+                    ..Default::default()
+                },
+                lsp::CompletionItem {
+                    label: "slice".into(),
+                    sort_text: Some("d".into()),
+                    ..Default::default()
+                },
+            ])))
+        });
+    cx.set_state("rˇ");
+    cx.executor().run_until_parked();
+    cx.update_editor(|editor, cx| {
+        editor.show_completions(
+            &ShowCompletions {
+                trigger: Some("r".into()),
+            },
+            cx,
+        );
+    });
+    cx.executor().run_until_parked();
+
+    cx.update_editor(|editor, _| {
+        if let Some(ContextMenu::Completions(menu)) = editor.context_menu.read().as_ref() {
+            assert_eq!(
+                menu.matches.iter().map(|m| &m.string).collect::<Vec<_>>(),
+                &["r", "ret", "Range", "return"]
+            );
+        } else {
+            panic!("expected completion menu to be open");
+        }
+    });
+}
+
 #[gpui::test]
 async fn test_no_duplicated_completion_requests(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});