From cc552dd70217cab9929c25b7b6304bc1e6e68bc3 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Tue, 9 Jun 2020 14:16:40 -0700 Subject: [PATCH] feat(language-service): Remove HTML entities autocompletion (#37515) This commit removes the autocompletion feature for HTML entities. HTML entites are things like `&`, `<` etc. There are a few reasons for the decision: 1. It is outside the core functionality of Angular LS 2. The implementation relies on regex, which incurs performance cost 3. There isn't much value if users do not already know which entity they want to use 4. The list that we provide is not exhaustive PR Close #37515 --- packages/language-service/src/completions.ts | 36 +------------------ .../language-service/test/completions_spec.ts | 14 -------- 2 files changed, 1 insertion(+), 49 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 4225fbaa18..f4fbdcb3f3 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AbsoluteSourceSpan, AST, AstPath, AttrAst, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, EmptyExpr, ExpressionBinding, getHtmlTagDefinition, HtmlAstPath, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ParseSpan, ReferenceAst, TagContentType, TemplateBinding, Text, VariableBinding} from '@angular/compiler'; +import {AbsoluteSourceSpan, AST, AstPath, AttrAst, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, EmptyExpr, ExpressionBinding, getHtmlTagDefinition, HtmlAstPath, Node as HtmlAst, NullTemplateVisitor, ParseSpan, ReferenceAst, TagContentType, TemplateBinding, Text, VariableBinding} from '@angular/compiler'; import {$$, $_, isAsciiLetter, isDigit} from '@angular/compiler/src/chars'; import {ATTR, getBindingDescriptor} from './binding_utils'; @@ -164,9 +164,6 @@ export function getTemplateCompletions( } }, visitText(ast) { - // Check if we are in a entity. - result = entityCompletions(getSourceText(template, spanOf(ast)), astPosition); - if (result.length) return result; result = interpolationCompletions(templateInfo, templatePosition); if (result.length) return result; const element = path.first(Element); @@ -359,33 +356,6 @@ function elementCompletions(info: ng.AstResult): ng.CompletionEntry[] { return results; } -function entityCompletions(value: string, position: number): ng.CompletionEntry[] { - // Look for entity completions - // TODO(kyliau): revisit the usefulness of this feature. It provides - // autocompletion for HTML entities, which IMO is outside the core functionality - // of Angular language service. Besides, we do not have a complete list. - // See https://dev.w3.org/html5/html-author/charref - const re = /&[A-Za-z]*;?(?!\d)/g; - let found: RegExpExecArray|null; - let result: ng.CompletionEntry[] = []; - while (found = re.exec(value)) { - let len = found[0].length; - // end position must be inclusive to account for cases like '&|' where - // cursor is right behind the ampersand. - if (position >= found.index && position <= (found.index + len)) { - result = Object.keys(NAMED_ENTITIES).map(name => { - return { - name: `&${name};`, - kind: ng.CompletionKind.ENTITY, - sortText: name, - }; - }); - break; - } - } - return result; -} - function interpolationCompletions(info: ng.AstResult, position: number): ng.CompletionEntry[] { // Look for an interpolation in at the position. const templatePath = findTemplateAstAt(info.templateAst, position); @@ -598,10 +568,6 @@ class ExpressionVisitor extends NullTemplateVisitor { } } -function getSourceText(template: ng.TemplateSource, span: ng.Span): string { - return template.source.substring(span.start, span.end); -} - interface AngularAttributes { /** * Attributes that support the * syntax. See https://angular.io/api/core/TemplateRef diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index d8fe23cd1f..ccece9379b 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -27,13 +27,6 @@ describe('completions', () => { mockHost.reset(); }); - it('should be able to get entity completions', () => { - mockHost.overrideInlineTemplate(APP_COMPONENT, '&~{cursor}'); - const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); - const completions = ngLS.getCompletionsAtPosition(APP_COMPONENT, marker.start); - expectContain(completions, CompletionKind.ENTITY, ['&', '>', '<', 'ι']); - }); - it('should be able to return html elements', () => { mockHost.overrideInlineTemplate(APP_COMPONENT, '<~{cursor}'); const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, 'cursor'); @@ -241,13 +234,6 @@ describe('completions', () => { }); describe('in external template', () => { - it('should be able to get entity completions in external template', () => { - mockHost.override(TEST_TEMPLATE, '&~{cursor}'); - const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); - const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); - expectContain(completions, CompletionKind.ENTITY, ['&', '>', '<', 'ι']); - }); - it('should not return html elements', () => { mockHost.override(TEST_TEMPLATE, '<~{cursor}'); const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor');