Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Emmet reduce JSX array noise #146757

Merged
merged 3 commits into from Apr 5, 2022
Merged

Emmet reduce JSX array noise #146757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77d3eac
Reduce emmet JSX array noise, fixes #138461
rzhao271 Apr 4, 2022
16dab03
Strengthen tests
rzhao271 Apr 4, 2022
140e36b
More polish
rzhao271 Apr 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 10 additions & 8 deletions extensions/emmet/src/defaultCompletionProvider.ts
View file
Open in desktop
Expand Up @@ -161,25 +161,27 @@ export class DefaultCompletionItemProvider implements vscode.CompletionItemProvi
return;
}

let noiseCheckPromise: Thenable<any> = Promise.resolve();
let isNoisePromise: Thenable<boolean> = 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.SymbolInformation[]>('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.SymbolInformation[] | undefined>('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!);
Expand Down
77 changes: 24 additions & 53 deletions extensions/emmet/src/test/completion.test.ts
View file
Open in desktop
Expand Up @@ -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('<div style="|"', [{ label: 'padding: ;' }]);
await testHtmlCompletionProvider(`<div style='|'`, [{ label: 'padding: ;' }]);
await testHtmlCompletionProvider(`<div style='p|'`, [{ label: 'padding: ;' }]);
await testHtmlCompletionProvider(`<div style='color: #0|'`, [{ label: '#000000' }]);
await testCompletionProvider('html', '<div style="|"', [{ label: 'padding: ;' }]);
await testCompletionProvider('html', `<div style='|'`, [{ label: 'padding: ;' }]);
await testCompletionProvider('html', `<div style='p|'`, [{ label: 'padding: ;' }]);
await testCompletionProvider('html', `<div style='color: #0|'`, [{ label: '#000000' }]);
});

// https://github.com/microsoft/vscode/issues/79766
test('#79766, correct region determination', async () => {
await testHtmlCompletionProvider(`<div style="color: #000">di|</div>`, [
await testCompletionProvider('html', `<div style="color: #000">di|</div>`, [
{ label: 'div', documentation: `<div>|</div>` }
]);
});

// 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: '<div a="b"></div>', documentation: '<div a="b">|</div>' }
]);
});
});

interface TestCompletionItem {
Expand All @@ -49,11 +57,11 @@ interface TestCompletionItem {
documentation?: string;
}

function testHtmlCompletionProvider(contents: string, expectedItems: TestCompletionItem[]): Thenable<any> {
function testCompletionProvider(fileExtension: string, contents: string, expectedItems: TestCompletionItem[] | undefined): Thenable<boolean> {
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();
Expand All @@ -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<any> {
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}`);
Expand Down