Skip to content

Commit aacc798

Browse files
committed
Fix completer tests (make token substitution case sensitive, otherwise
selecting `ValueError` on `va` would produce `valueError`.
1 parent d90e671 commit aacc798

File tree

3 files changed

+28
-5
lines changed

3 files changed

+28
-5
lines changed

atest/05_Features/Completion.robot

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ Continuous Hinting Works
150150
[Setup] Prepare File for Editing Python completion completion.py
151151
Configure JupyterLab Plugin {"continuousHinting": true} plugin id=${COMPLETION PLUGIN ID}
152152
# TODO: remove once we resolve https://github.com/jupyterlab/jupyterlab/issues/15022
153-
Configure JupyterLab Plugin {"autoCompletion": true} plugin id=${MANAGER PLUGIN ID}
153+
Configure JupyterLab Plugin {"autoCompletion": true, "providerTimeout": 2500} plugin id=${MANAGER PLUGIN ID}
154154
Place Cursor In File Editor At 9 2
155155
Wait For Ready State
156156
Press Keys None d
@@ -373,6 +373,8 @@ Completes Paths In Strings
373373
*** Keywords ***
374374
Setup Completion Test
375375
Setup Notebook Python Completion.ipynb
376+
# TODO: this should be per-provider (upstream issue)
377+
Configure JupyterLab Plugin {"providerTimeout": 2500} plugin id=${MANAGER PLUGIN ID}
376378

377379
Get Cell Editor Content
378380
[Arguments] ${cell_nr}

packages/jupyterlab-lsp/src/features/completion/model.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,28 @@ export class GenericCompleterModel<
6565
if (this.current && this.cursor) {
6666
// set initial query to pre-filter items; in future we should use:
6767
// https://github.com/jupyterlab/jupyterlab/issues/9763#issuecomment-1001603348
68+
69+
// note: start/end from cursor are not ideal because these get populated from fetch
70+
// reply which will vary depending on what providers decide to return; we want the
71+
// actual position in token, the same as passed in request to fetch. We can get it
72+
// by searching for longest common prefix as seen below (or by counting characters).
73+
// Maybe upstream should expose it directly?
6874
const { start, end } = this.cursor;
69-
let query = this.current.text.substring(start, end).trim();
75+
const { text, line, column } = this.original!;
76+
77+
const queryRange = text.substring(start, end).trim();
78+
const linePrefix = text.split('\n')[line].substring(0, column).trim();
79+
let query = '';
80+
for (let i = queryRange.length; i > 0; i--) {
81+
if (queryRange.slice(0, i) == linePrefix.slice(-i)) {
82+
query = linePrefix.slice(-i);
83+
break;
84+
}
85+
}
86+
if (!query) {
87+
return;
88+
}
89+
7090
let trimmedQuotes = false;
7191
// special case for "Completes Paths In Strings" test case
7292
if (query.startsWith('"') || query.startsWith("'")) {

packages/jupyterlab-lsp/src/features/completion/provider.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,10 +343,10 @@ export function transformLSPCompletions<T>(
343343
}
344344

345345
// Add overlap with token prefix
346-
if (intendedText.toLowerCase().startsWith(token.value.toLowerCase())) {
346+
if (intendedText.startsWith(token.value)) {
347347
anyPrefixed = true;
348348
// remove overlap with prefix before expanding it
349-
if (intendedText.toLowerCase().startsWith(prefix.toLowerCase())) {
349+
if (intendedText.startsWith(prefix)) {
350350
text = text.substring(prefix.length, text.length);
351351
match.insertText = text;
352352
}
@@ -364,7 +364,8 @@ export function transformLSPCompletions<T>(
364364
text =
365365
(prefix.startsWith("'") || prefix.startsWith('"') ? prefix[0] : '') +
366366
pathPrefix +
367-
text;
367+
text +
368+
(suffix.startsWith("'") || suffix.startsWith('"') ? suffix[0] : '');
368369
match.insertText = text;
369370
// for label without quotes
370371
match.label = pathPrefix + match.label;

0 commit comments

Comments
 (0)