Skip to content

Commit 2349d21

Browse files
authored
improve completions sorting (#798)
* Try to manually improve completions sorting * filter namespaces in completions * add support for tab for accepting completions * Remove namespace filtering * remove code that disabled completions in Positron
1 parent 3fe790b commit 2349d21

File tree

4 files changed

+165
-128
lines changed

4 files changed

+165
-128
lines changed

apps/vscode/src/providers/editor/codeview.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,6 @@ export function vscodeCodeViewServer(_engine: MarkdownEngine, document: TextDocu
102102
return lspCellYamlOptionsCompletions(context, lspRequest);
103103
}
104104

105-
// if this is Positron, no visual editor completions
106-
// TODO: fix LSP issues for visual editor in Positron:
107-
// https://github.com/posit-dev/positron/issues/1805
108-
if (hasHooks()) {
109-
return {
110-
items: [],
111-
isIncomplete: false
112-
};
113-
}
114-
115105
// otherwise delegate to vscode completion system
116106
const vdoc = virtualDocForCode(context.code, language);
117107
const completions = await vdocCompletions(

packages/editor-codemirror/src/behaviors/completion.ts

Lines changed: 148 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -102,135 +102,160 @@ export function completionBehavior(behaviorContext: BehaviorContext): Behavior {
102102
};
103103
}
104104

105-
async function getCompletions(
106-
context: CompletionContext,
107-
cvContext: CodeViewCompletionContext,
108-
behaviorContext: BehaviorContext
109-
): Promise<CompletionResult | null> {
110-
111-
// get completions
112-
const completions = await behaviorContext.pmContext.ui.codeview?.codeViewCompletions(cvContext);
113-
if (context.aborted || !completions || completions.items.length == 0) {
114-
return null;
105+
const compareBySortText = (a: CompletionItem, b: CompletionItem) => {
106+
if (a.sortText && b.sortText) {
107+
return a.sortText.localeCompare(b.sortText);
108+
} else {
109+
return 0;
115110
}
111+
};
116112

117-
// order completions
118-
const haveOrder = !!completions.items?.[0].sortText;
119-
if (haveOrder) {
120-
completions.items = completions.items.sort((a, b) => {
121-
if (a.sortText && b.sortText) {
122-
return a.sortText.localeCompare(b.sortText);
123-
} else {
124-
return 0;
113+
// compute from
114+
const itemFrom = (item: CompletionItem, contextPos: number) => {
115+
// compute from
116+
return item.textEdit
117+
? InsertReplaceEdit.is(item.textEdit)
118+
? contextPos - (item.textEdit.insert.end.character - item.textEdit.insert.start.character)
119+
: TextEdit.is(item.textEdit)
120+
? contextPos - (item.textEdit.range.end.character - item.textEdit.range.start.character)
121+
: contextPos
122+
: contextPos;
123+
};
124+
125+
/**
126+
* replaceText for a given CompletionItem is the text that is already in the document
127+
* that that CompletionItem will replace.
128+
*
129+
* Example 1: if you are typing `lib` and get the completion `library`, then this function
130+
* will give `lib`.
131+
* Example 2: if you are typing `os.a` and get the completion `abc`, then this function
132+
* will give `a`.
133+
*/
134+
const getReplaceText = (context: CompletionContext, item: CompletionItem) =>
135+
context.state.sliceDoc(itemFrom(item, context.pos), context.pos);
136+
137+
const makeCompletionItemApplier = (item: CompletionItem, context: CompletionContext) =>
138+
(view: EditorView, completion: Completion) => {
139+
// compute from
140+
const from = itemFrom(item, context.pos);
141+
142+
// handle snippets
143+
const insertText = item.textEdit?.newText ?? (item.insertText || item.label);
144+
if (item.insertTextFormat === InsertTextFormat.Snippet) {
145+
const insertSnippet = snippet(insertText.replace(/\$(\d+)/g, "$${$1}"));
146+
insertSnippet(view, completion, from, context.pos);
147+
// normal completions
148+
} else {
149+
view.dispatch({
150+
...insertCompletionText(view.state, insertText, from, context.pos),
151+
annotations: pickedCompletion.of(completion)
152+
});
153+
if (item.command?.command === "editor.action.triggerSuggest") {
154+
startCompletion(view);
125155
}
126-
});
156+
}
157+
};
158+
159+
const sortTextItemsBoostScore = (context: CompletionContext, items: CompletionItem[], index: number) => {
160+
const total = items.length;
161+
const item = items[index];
162+
// compute replaceText
163+
const replaceText = getReplaceText(context, item);
164+
165+
// if the replaceText doesn't start with "." then bury items that do
166+
if (!replaceText.startsWith(".") && item.label.startsWith(".")) {
167+
return -99;
127168
}
128169

129-
// compute token
130-
const token = context.matchBefore(/\S+/)?.text;
170+
// only boost things that have a prefix match
171+
if (item.label.toLowerCase().startsWith(replaceText) ||
172+
(item.textEdit && item.textEdit.newText.toLowerCase().startsWith(replaceText)) ||
173+
(item.insertText && item.insertText.toLowerCase().startsWith(replaceText))) {
174+
return -99 + Math.round(((total - index) / total) * 198);;
175+
} else {
176+
return -99;
177+
}
178+
};
131179

132-
// compute from
133-
const itemFrom = (item: CompletionItem) => {
134-
// compute from
135-
return item.textEdit
136-
? InsertReplaceEdit.is(item.textEdit)
137-
? context.pos - (item.textEdit.insert.end.character - item.textEdit.insert.start.character)
138-
: TextEdit.is(item.textEdit)
139-
? context.pos - (item.textEdit.range.end.character - item.textEdit.range.start.character)
140-
: context.pos
141-
: context.pos;
142-
};
180+
const defaultBoostScore = (context: CompletionContext, items: CompletionItem[], index: number) => {
181+
const item = items[index];
143182

144-
// use order to create boost
145-
const total = completions.items.length;
146-
const boostScore = (index: number) => {
183+
const replaceText = getReplaceText(context, item);
147184

148-
// compute replaceText
149-
const item = completions.items[index];
150-
const replaceText = context.state.sliceDoc(itemFrom(item), context.pos).toLowerCase();
185+
// if you haven't typed into the completions yet (for example after a `.`) then
186+
// score items starting with non-alphabetic characters -1, everything else 0.
187+
if (replaceText.length === 0) return isLetter(item.label[0]) ? 0 : -1;
151188

152-
if (haveOrder) {
189+
// We filter items by replaceText inclusion before scoring,
190+
// so i is garaunteed to be an index into `item.label`...
191+
const i = item.label.toLowerCase().indexOf(replaceText.toLowerCase());
192+
// and `replaceTextInItermLabel` should be the same as `replaceText` up to upper/lowercase
193+
// differences.
194+
const replaceTextInItemLabel = item.label.slice(i, replaceText.length);
153195

154-
// if the replaceText doesn't start with "." then bury items that do
155-
if (!replaceText.startsWith(".") && item.label.startsWith(".")) {
156-
return -99;
157-
}
196+
// mostly counts how many upper/lowercase differences there are
197+
let diff = simpleStringDiff(replaceTextInItemLabel, replaceText);
158198

159-
// only boost things that have a prefix match
160-
if (item.label.toLowerCase().startsWith(replaceText) ||
161-
(item.textEdit && item.textEdit.newText.toLowerCase().startsWith(replaceText)) ||
162-
(item.insertText && item.insertText.toLowerCase().startsWith(replaceText))) {
163-
return -99 + Math.round(((total - index) / total) * 198);;
164-
} else {
165-
return -99;
166-
}
199+
// `-i` scores completions better if what you typed is earlier in the completion
200+
// `-diff/10` mostly tie breaks that score by capitalization differences.
201+
return -i - diff / 10; // 10 is a magic number
202+
};
167203

168-
} else {
169-
return undefined;
170-
}
171-
};
204+
async function getCompletions(
205+
context: CompletionContext,
206+
cvContext: CodeViewCompletionContext,
207+
behaviorContext: BehaviorContext
208+
): Promise<CompletionResult | null> {
209+
if (context.aborted) return null;
172210

173-
// return completions
174-
return {
175-
from: context.pos,
211+
// get completions
212+
const completions = await behaviorContext.pmContext.ui.codeview?.codeViewCompletions(cvContext);
213+
if (completions === undefined) return null;
214+
if (completions.items.length == 0) return null;
176215

177-
options: completions.items
178-
.filter(item => {
216+
const itemsHaveSortText = completions.items?.[0].sortText !== undefined;
179217

180-
// no text completions that aren't snippets
181-
if (item.kind === CompletionItemKind.Text &&
182-
item.insertTextFormat !== InsertTextFormat.Snippet) {
183-
return false;
184-
}
218+
const items = itemsHaveSortText ?
219+
completions.items.sort(compareBySortText) :
220+
completions.items;
185221

186-
// compute text to replace
187-
const replaceText = context.state.sliceDoc(itemFrom(item), context.pos).toLowerCase();
222+
// The token is the contents of the line up to your cursor.
223+
// For example, if you type `os.a` then token will be `os.a`.
224+
// Note: in contrast, when you type `os.a` replaceText will give `a` for a completion like `abc`.
225+
const token = context.matchBefore(/\S+/)?.text;
188226

189-
// only allow non-text edits if we have no token
190-
if (!item.textEdit && token) {
191-
return false;
192-
}
227+
const filteredItems = items.filter(item => {
228+
// no text completions that aren't snippets
229+
if (item.kind === CompletionItemKind.Text &&
230+
item.insertTextFormat !== InsertTextFormat.Snippet) return false;
231+
232+
// only allow non-text edits if we have no token
233+
if (item.textEdit === undefined && token) return false;
234+
235+
// require at least inclusion
236+
const replaceText = getReplaceText(context, item).toLowerCase();
237+
return item.label.toLowerCase().includes(replaceText) ||
238+
item.insertText?.toLowerCase().includes(replaceText);
239+
});
240+
241+
const boostScore = itemsHaveSortText ?
242+
sortTextItemsBoostScore :
243+
defaultBoostScore;
244+
245+
const options = filteredItems
246+
.map((item, index): Completion => {
247+
return {
248+
label: item.label,
249+
detail: !item.documentation ? item.detail : undefined,
250+
type: vsKindToType(item.kind),
251+
info: () => infoNodeForItem(item),
252+
apply: makeCompletionItemApplier(item, context),
253+
boost: boostScore(context, filteredItems, index)
254+
};
255+
});
193256

194-
// require at least inclusion
195-
return item.label.toLowerCase().includes(replaceText) ||
196-
(item.insertText && item.insertText.toLowerCase().includes(replaceText));
197-
})
198-
.map((item, index): Completion => {
199-
return {
200-
label: item.label,
201-
detail: item.detail && !item.documentation ? item.detail : undefined,
202-
type: vsKindToType(item.kind),
203-
info: (): Node | null => {
204-
if (item.documentation) {
205-
return infoNodeForItem(item);
206-
} else {
207-
return null;
208-
}
209-
},
210-
apply: (view: EditorView, completion: Completion, from: number) => {
211-
// compute from
212-
from = itemFrom(item);
213-
214-
// handle snippets
215-
const insertText = item.textEdit?.newText ?? (item.insertText || item.label);
216-
if (item.insertTextFormat === InsertTextFormat.Snippet) {
217-
const insertSnippet = snippet(insertText.replace(/\$(\d+)/g, "$${$1}"));
218-
insertSnippet(view, completion, from, context.pos);
219-
// normal completions
220-
} else {
221-
view.dispatch({
222-
...insertCompletionText(view.state, insertText, from, context.pos),
223-
annotations: pickedCompletion.of(completion)
224-
});
225-
if (item.command?.command === "editor.action.triggerSuggest") {
226-
startCompletion(view);
227-
}
228-
}
229-
},
230-
boost: boostScore(index)
231-
};
232-
})
233-
};
257+
// return completions
258+
return { from: context.pos, options };
234259
}
235260

236261

@@ -281,7 +306,6 @@ function vsKindToType(kind?: CompletionItemKind) {
281306

282307

283308
function infoNodeForItem(item: CompletionItem) {
284-
285309
const headerEl = (text: string, tag: string) => {
286310
const header = document.createElement(tag);
287311
header.classList.add("cm-completionInfoHeader");
@@ -328,3 +352,15 @@ function infoNodeForItem(item: CompletionItem) {
328352
return null;
329353
}
330354
}
355+
356+
function simpleStringDiff(str1: string, str2: string) {
357+
let diff = 0;
358+
for (let i = 0; i < Math.min(str1.length, str2.length); i++) {
359+
if (str1[i] !== str2[i]) diff++;
360+
}
361+
return diff;
362+
};
363+
364+
function isLetter(c: string) {
365+
return c.toLowerCase() != c.toUpperCase();
366+
}

packages/editor-codemirror/src/behaviors/indent.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,29 @@
1313
*
1414
*/
1515

16-
import { indentWithTab } from "@codemirror/commands";
16+
import { indentLess, indentMore } from "@codemirror/commands";
1717

1818
import { indentOnInput } from "@codemirror/language";
1919
import { keymap } from "@codemirror/view";
2020

2121
import { Behavior } from ".";
22+
import { acceptCompletion, completionStatus } from "@codemirror/autocomplete";
2223

23-
export function indentBehavior() : Behavior {
24+
export function tabBehavior(): Behavior {
2425
return {
2526
extensions: [
2627
indentOnInput(),
27-
keymap.of([indentWithTab])
28+
keymap.of([
29+
{
30+
key: 'Tab',
31+
preventDefault: true,
32+
shift: indentLess,
33+
run: e => {
34+
if (!completionStatus(e.state)) return indentMore(e);
35+
return acceptCompletion(e);
36+
},
37+
},
38+
])
2839
]
29-
}
40+
};
3041
}

packages/editor-codemirror/src/behaviors/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { CodeViewOptions, ExtensionContext } from "editor";
2727
import { langModeBehavior } from './langmode';
2828
import { keyboardBehavior } from './keyboard';
2929
import { findBehavior } from './find';
30-
import { indentBehavior } from './indent';
30+
import { tabBehavior } from './indent';
3131
import { trackSelectionBehavior } from './trackselection';
3232
import { themeBehavior } from './theme';
3333
import { prefsBehavior } from './prefs';
@@ -63,7 +63,7 @@ export function createBehaviors(context: BehaviorContext): Behavior[] {
6363
langModeBehavior(context),
6464
completionBehavior(context),
6565
findBehavior(context),
66-
indentBehavior(),
66+
tabBehavior(),
6767
themeBehavior(context),
6868
prefsBehavior(context),
6969
trackSelectionBehavior(context),

0 commit comments

Comments
 (0)