From 77d3eac74dd58d0d5e1116139c1d593892f0e25e Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 4 Apr 2022 10:38:36 -0700 Subject: [PATCH 1/3] Reduce emmet JSX array noise, fixes #138461 --- .../emmet/src/defaultCompletionProvider.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/extensions/emmet/src/defaultCompletionProvider.ts b/extensions/emmet/src/defaultCompletionProvider.ts index afb90b64418a1..5c187bed6d17d 100644 --- a/extensions/emmet/src/defaultCompletionProvider.ts +++ b/extensions/emmet/src/defaultCompletionProvider.ts @@ -161,25 +161,27 @@ export class DefaultCompletionItemProvider implements vscode.CompletionItemProvi return; } - let noiseCheckPromise: Thenable = Promise.resolve(); + let isNoisePromise: Thenable = Promise.resolve(false); // Fix for https://github.com/microsoft/vscode/issues/32647 // Check for document symbols in js/ts/jsx/tsx and avoid triggering emmet for abbreviations of the form symbolName.sometext // Presence of > or * or + in the abbreviation denotes valid abbreviation that should trigger emmet if (!isStyleSheet(syntax) && (document.languageId === 'javascript' || document.languageId === 'javascriptreact' || document.languageId === 'typescript' || document.languageId === 'typescriptreact')) { let abbreviation: string = extractAbbreviationResults.abbreviation; - if (abbreviation.startsWith('this.')) { - noiseCheckPromise = Promise.resolve(true); + // For the second condition, we don't want abbreviations that have [] characters but not ='s in them to expand + // In turn, users must explicitly expand abbreviations of the form Component[attr1 attr2], but it means we don't try to expand a[i]. + if (abbreviation.startsWith('this.') || /\[[^\]=]*\]/.test(abbreviation)) { + isNoisePromise = Promise.resolve(true); } else { - noiseCheckPromise = vscode.commands.executeCommand('vscode.executeDocumentSymbolProvider', document.uri).then((symbols: vscode.SymbolInformation[] | undefined) => { - return symbols && symbols.find(x => abbreviation === x.name || (abbreviation.startsWith(x.name + '.') && !/>|\*|\+/.test(abbreviation))); + isNoisePromise = vscode.commands.executeCommand('vscode.executeDocumentSymbolProvider', document.uri).then(symbols => { + return !!symbols && symbols?.some(x => abbreviation === x.name || (abbreviation.startsWith(x.name + '.') && !/>|\*|\+/.test(abbreviation))); }); } } - return noiseCheckPromise.then((noise): vscode.CompletionList | undefined => { - if (noise) { - return; + return isNoisePromise.then((isNoise): vscode.CompletionList | undefined => { + if (isNoise) { + return undefined; } const config = getEmmetConfiguration(syntax!); From 16dab03ca5fbecb40d79dc760c219ad41cead915 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 4 Apr 2022 11:02:19 -0700 Subject: [PATCH 2/3] Strengthen tests --- extensions/emmet/src/test/completion.test.ts | 77 ++++++-------------- 1 file changed, 24 insertions(+), 53 deletions(-) diff --git a/extensions/emmet/src/test/completion.test.ts b/extensions/emmet/src/test/completion.test.ts index 755b11aea18dd..ec94a0f49281d 100644 --- a/extensions/emmet/src/test/completion.test.ts +++ b/extensions/emmet/src/test/completion.test.ts @@ -15,32 +15,40 @@ suite('Tests for completion in CSS embedded in HTML', () => { teardown(closeAllEditors); test('style attribute & attribute value in html', async () => { - await testHtmlCompletionProvider('
{ - await testHtmlCompletionProvider(`
di|
`, [ + await testCompletionProvider('html', `
di|
`, [ { label: 'div', documentation: `
|
` } ]); }); // https://github.com/microsoft/vscode/issues/86941 test('#86941, widows should not be completed', async () => { - await testCssCompletionProvider(`.foo { wi| }`, [ - { label: 'widows: ;', documentation: `widows: ;` } - ]); + await testCompletionProvider('css', `.foo { wi| }`, undefined); }); // https://github.com/microsoft/vscode/issues/117020 test('#117020, ! at end of abbreviation should have completion', async () => { - await testCssCompletionProvider(`.foo { bdbn!| }`, [ + await testCompletionProvider('css', `.foo { bdbn!| }`, [ { label: 'border-bottom: none !important;', documentation: `border-bottom: none !important;` } ]); }); + + // https://github.com/microsoft/vscode/issues/138461 + test('#138461, JSX array noise', async () => { + await testCompletionProvider('jsx', 'a[i]', undefined); + await testCompletionProvider('jsx', 'Component[a b]', undefined); + await testCompletionProvider('jsx', '[a, b]', undefined); + await testCompletionProvider('jsx', '[a=b]', [ + { label: '
', documentation: '
|
' } + ]); + }); }); interface TestCompletionItem { @@ -49,11 +57,11 @@ interface TestCompletionItem { documentation?: string; } -function testHtmlCompletionProvider(contents: string, expectedItems: TestCompletionItem[]): Thenable { +function testCompletionProvider(fileExtension: string, contents: string, expectedItems: TestCompletionItem[] | undefined): Thenable { const cursorPos = contents.indexOf('|'); - const htmlContents = contents.slice(0, cursorPos) + contents.slice(cursorPos + 1); + const slicedContents = contents.slice(0, cursorPos) + contents.slice(cursorPos + 1); - return withRandomFileEditor(htmlContents, 'html', async (editor, _doc) => { + return withRandomFileEditor(slicedContents, fileExtension, async (editor, _doc) => { const selection = new Selection(editor.document.positionAt(cursorPos), editor.document.positionAt(cursorPos)); editor.selection = selection; const cancelSrc = new CancellationTokenSource(); @@ -69,51 +77,14 @@ function testHtmlCompletionProvider(contents: string, expectedItems: TestComplet const completionList = await completionPromise; if (!completionList || !completionList.items || !completionList.items.length) { - return Promise.resolve(); - } - - expectedItems.forEach(eItem => { - const matches = completionList.items.filter(i => i.label === eItem.label); - const match = matches && matches.length > 0 ? matches[0] : undefined; - assert.ok(match, `Didn't find completion item with label ${eItem.label}`); - - if (match) { - assert.strictEqual(match.detail, 'Emmet Abbreviation', `Match needs to come from Emmet`); - - if (eItem.documentation) { - assert.strictEqual(match.documentation, eItem.documentation, `Emmet completion Documentation doesn't match`); - } + if (completionList === undefined) { + assert.strictEqual(expectedItems, completionList); } - }); - - return Promise.resolve(); - }); -} - -function testCssCompletionProvider(contents: string, expectedItems: TestCompletionItem[]): Thenable { - const cursorPos = contents.indexOf('|'); - const cssContents = contents.slice(0, cursorPos) + contents.slice(cursorPos + 1); - - return withRandomFileEditor(cssContents, 'css', async (editor, _doc) => { - const selection = new Selection(editor.document.positionAt(cursorPos), editor.document.positionAt(cursorPos)); - editor.selection = selection; - const cancelSrc = new CancellationTokenSource(); - const completionPromise = completionProvider.provideCompletionItems( - editor.document, - editor.selection.active, - cancelSrc.token, - { triggerKind: CompletionTriggerKind.Invoke, triggerCharacter: undefined } - ); - if (!completionPromise) { - return Promise.resolve(); - } - - const completionList = await completionPromise; - if (!completionList || !completionList.items || !completionList.items.length) { return Promise.resolve(); } - expectedItems.forEach(eItem => { + assert.strictEqual(expectedItems === undefined, false); + expectedItems!.forEach(eItem => { const matches = completionList.items.filter(i => i.label === eItem.label); const match = matches && matches.length > 0 ? matches[0] : undefined; assert.ok(match, `Didn't find completion item with label ${eItem.label}`); From 140e36b8037f700351483424223315994e74fa25 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Tue, 5 Apr 2022 11:44:29 -0700 Subject: [PATCH 3/3] More polish --- extensions/emmet/src/defaultCompletionProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/emmet/src/defaultCompletionProvider.ts b/extensions/emmet/src/defaultCompletionProvider.ts index 5c187bed6d17d..1462a2ec4d78c 100644 --- a/extensions/emmet/src/defaultCompletionProvider.ts +++ b/extensions/emmet/src/defaultCompletionProvider.ts @@ -174,7 +174,7 @@ export class DefaultCompletionItemProvider implements vscode.CompletionItemProvi isNoisePromise = Promise.resolve(true); } else { isNoisePromise = vscode.commands.executeCommand('vscode.executeDocumentSymbolProvider', document.uri).then(symbols => { - return !!symbols && symbols?.some(x => abbreviation === x.name || (abbreviation.startsWith(x.name + '.') && !/>|\*|\+/.test(abbreviation))); + return !!symbols && symbols.some(x => abbreviation === x.name || (abbreviation.startsWith(x.name + '.') && !/>|\*|\+/.test(abbreviation))); }); } }