diff --git a/CHANGELOG.md b/CHANGELOG.md index 753955d53..99bfa8bc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ - implement settings UI using native JupyterLab 3.3 UI ([#778]) - bug fixes - use correct websocket URL if configured as different from base URL ([#820], thanks @MikeSem) - - clean up all completer styles when compelter feature is disabled ([#829]). + - clean up all completer styles when completer feature is disabled ([#829]). + - fix `undefined` being inserted for path-like completion items with no `insertText` ([#833]) - refactoring: - move client capabilities to features ([#738]) - documentation: @@ -28,6 +29,7 @@ [#820]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/820 [#826]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/826 [#829]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/829 +[#833]: https://github.com/jupyter-lsp/jupyterlab-lsp/pull/833 ### `@krassowski/jupyterlab-lsp 3.10.1` (2022-03-21) diff --git a/packages/jupyterlab-lsp/src/features/completion/completion_handler.spec.ts b/packages/jupyterlab-lsp/src/features/completion/completion_handler.spec.ts new file mode 100644 index 000000000..537323494 --- /dev/null +++ b/packages/jupyterlab-lsp/src/features/completion/completion_handler.spec.ts @@ -0,0 +1,87 @@ +import { CodeEditor } from '@jupyterlab/codeeditor'; +import { expect } from 'chai'; +import type * as lsProtocol from 'vscode-languageserver-types'; + +import { BrowserConsole } from '../../virtual/console'; + +import { transformLSPCompletions } from './completion_handler'; + +describe('transformLSPCompletions', () => { + const browserConsole = new BrowserConsole(); + + const transform = ( + token: CodeEditor.IToken, + position: number, + completions: lsProtocol.CompletionItem[] + ) => { + return transformLSPCompletions( + token, + position, + completions, + (kind: string, match: lsProtocol.CompletionItem) => { + return { kind, match }; + }, + browserConsole + ); + }; + + it('Harmonizes `insertText` from `undefined` when adjusting path-like completions', () => { + // `import { } from '@lumino/'` + const result = transform( + { + offset: 16, + value: "'@lumino/'", + type: 'string' + }, + 8, + [ + { + label: 'algorithm', + kind: 19, + sortText: '1', + data: { + entryNames: ['algorithm'] + } + }, + { + label: 'application', + kind: 19, + sortText: '1', + data: { + entryNames: ['application'] + } + } + ] + ); + expect(result.items.length).to.equal(2); + // insert text should keep `'` as it was part of original token + expect(result.items[0].match.insertText).to.equal("'@lumino/algorithm"); + // label should not include `'` + expect(result.items[0].match.label).to.equal('@lumino/algorithm'); + expect(result.source.name).to.equal('LSP'); + }); + + it('Harmonizes `insertText` for paths', () => { + // `'./Hov` + const result = transform( + { + offset: 0, + value: "'./Hov", + type: 'string' + }, + 5, + [ + { + label: "Hover.ipynb'", + kind: 17, + sortText: "aHover.ipynb'", + insertText: "Hover.ipynb'" + } + ] + ); + // insert text should keep `'` as it was part of original token + expect(result.items[0].match.insertText).to.equal("'./Hover.ipynb'"); + // label should not include initial `'` + expect(result.items[0].match.label).to.equal("./Hover.ipynb'"); + }); +}); diff --git a/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts b/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts index e13c2f19a..1693590ec 100644 --- a/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts +++ b/packages/jupyterlab-lsp/src/features/completion/completion_handler.ts @@ -12,7 +12,7 @@ import { KernelKind } from '@krassowski/completion-theme/lib/types'; import { JSONArray, JSONObject } from '@lumino/coreutils'; -import * as lsProtocol from 'vscode-languageserver-types'; +import type * as lsProtocol from 'vscode-languageserver-types'; import { CodeCompletion as LSPCompletionSettings } from '../../_completion'; import { LSPConnection } from '../../connection'; @@ -53,6 +53,99 @@ export interface ICompletionsReply items: IExtendedCompletionItem[]; } +export function transformLSPCompletions( + token: CodeEditor.IToken, + position_in_token: number, + lspCompletionItems: lsProtocol.CompletionItem[], + createCompletionItem: (kind: string, match: lsProtocol.CompletionItem) => T, + console: ILSPLogConsole +) { + let prefix = token.value.slice(0, position_in_token + 1); + let all_non_prefixed = true; + let items: T[] = []; + lspCompletionItems.forEach(match => { + let kind = match.kind ? CompletionItemKind[match.kind] : ''; + + // Update prefix values + let text = match.insertText ? match.insertText : match.label; + + // declare prefix presence if needed and update it + if (text.toLowerCase().startsWith(prefix.toLowerCase())) { + all_non_prefixed = false; + if (prefix !== token.value) { + if (text.toLowerCase().startsWith(token.value.toLowerCase())) { + // given a completion insert text "display_table" and two test cases: + // dispdata → display_tabledata + // display → display_table + // we have to adjust the prefix for the latter (otherwise we would get display_tablelay), + // as we are constrained NOT to replace after the prefix (which would be "disp" otherwise) + prefix = token.value; + } + } + } + // add prefix if needed + else if (token.type === 'string' && prefix.includes('/')) { + // special case for path completion in strings, ensuring that: + // '/Com → '/Completion.ipynb + // when the returned insert text is `Completion.ipynb` (the token here is `'/Com`) + // developed against pyls and pylsp server, may not work well in other cases + const parts = prefix.split('/'); + if ( + text.toLowerCase().startsWith(parts[parts.length - 1].toLowerCase()) + ) { + let pathPrefix = parts.slice(0, -1).join('/') + '/'; + match.insertText = pathPrefix + text; + // for label removing the prefix quote if present + if (pathPrefix.startsWith("'") || pathPrefix.startsWith('"')) { + pathPrefix = pathPrefix.substr(1); + } + match.label = pathPrefix + match.label; + all_non_prefixed = false; + } + } + + let completionItem = createCompletionItem(kind, match); + + items.push(completionItem); + }); + console.debug('Transformed'); + // required to make the repetitive trigger characters like :: or ::: work for R with R languageserver, + // see https://github.com/jupyter-lsp/jupyterlab-lsp/issues/436 + let prefix_offset = token.value.length; + // completion of dictionaries for Python with jedi-language-server was + // causing an issue for dic[''] case; to avoid this let's make + // sure that prefix.length >= prefix.offset + if (all_non_prefixed && prefix_offset > prefix.length) { + prefix_offset = prefix.length; + } + + let response = { + // note in the ContextCompleter it was: + // start: token.offset, + // end: token.offset + token.value.length, + // which does not work with "from statistics import " as the last token ends at "t" of "import", + // so the completer would append "mean" as "from statistics importmean" (without space!); + // (in such a case the typedCharacters is undefined as we are out of range) + // a different workaround would be to prepend the token.value prefix: + // text = token.value + text; + // but it did not work for "from statistics " and lead to "from statisticsimport" (no space) + start: token.offset + (all_non_prefixed ? prefix_offset : 0), + end: token.offset + prefix.length, + items: items, + source: { + name: 'LSP', + priority: 2 + } + }; + if (response.start > response.end) { + console.warn( + 'Response contains start beyond end; this should not happen!', + response + ); + } + return response; +} + /** * A LSP connector for completion handlers. */ @@ -376,97 +469,21 @@ export class LSPConnector )) || []) as lsProtocol.CompletionItem[]; this.console.debug('Transforming'); - let prefix = token.value.slice(0, position_in_token + 1); - let all_non_prefixed = true; - let items: IExtendedCompletionItem[] = []; - lspCompletionItems.forEach(match => { - let kind = match.kind ? CompletionItemKind[match.kind] : ''; - - // Update prefix values - let text = match.insertText ? match.insertText : match.label; - - // declare prefix presence if needed and update it - if (text.toLowerCase().startsWith(prefix.toLowerCase())) { - all_non_prefixed = false; - if (prefix !== token.value) { - if (text.toLowerCase().startsWith(token.value.toLowerCase())) { - // given a completion insert text "display_table" and two test cases: - // dispdata → display_tabledata - // display → display_table - // we have to adjust the prefix for the latter (otherwise we would get display_tablelay), - // as we are constrained NOT to replace after the prefix (which would be "disp" otherwise) - prefix = token.value; - } - } - } - // add prefix if needed - else if (token.type === 'string' && prefix.includes('/')) { - // special case for path completion in strings, ensuring that: - // '/Com → '/Completion.ipynb - // when the returned insert text is `Completion.ipynb` (the token here is `'/Com`) - // developed against pyls and pylsp server, may not work well in other cases - const parts = prefix.split('/'); - if ( - text.toLowerCase().startsWith(parts[parts.length - 1].toLowerCase()) - ) { - let pathPrefix = parts.slice(0, -1).join('/') + '/'; - match.insertText = pathPrefix + match.insertText; - // for label removing the prefix quote if present - if (pathPrefix.startsWith("'") || pathPrefix.startsWith('"')) { - pathPrefix = pathPrefix.substr(1); - } - match.label = pathPrefix + match.label; - all_non_prefixed = false; - } - } - - let completionItem = new LazyCompletionItem( - kind, - this.icon_for(kind), - match, - this, - document.uri - ); - items.push(completionItem); - }); - this.console.debug('Transformed'); - // required to make the repetitive trigger characters like :: or ::: work for R with R languageserver, - // see https://github.com/jupyter-lsp/jupyterlab-lsp/issues/436 - let prefix_offset = token.value.length; - // completion of dictionaries for Python with jedi-language-server was - // causing an issue for dic[''] case; to avoid this let's make - // sure that prefix.length >= prefix.offset - if (all_non_prefixed && prefix_offset > prefix.length) { - prefix_offset = prefix.length; - } - - let response = { - // note in the ContextCompleter it was: - // start: token.offset, - // end: token.offset + token.value.length, - // which does not work with "from statistics import " as the last token ends at "t" of "import", - // so the completer would append "mean" as "from statistics importmean" (without space!); - // (in such a case the typedCharacters is undefined as we are out of range) - // a different workaround would be to prepend the token.value prefix: - // text = token.value + text; - // but it did not work for "from statistics " and lead to "from statisticsimport" (no space) - start: token.offset + (all_non_prefixed ? prefix_offset : 0), - end: token.offset + prefix.length, - items: items, - source: { - name: 'LSP', - priority: 2 - } - }; - if (response.start > response.end) { - console.warn( - 'Response contains start beyond end; this should not happen!', - response - ); - } - - return response; + return transformLSPCompletions( + token, + position_in_token, + lspCompletionItems, + (kind, match) => + new LazyCompletionItem( + kind, + this.icon_for(kind), + match, + this, + document.uri + ), + this.console + ); } protected icon_for(type: string): LabIcon {