fix(language-service): Preserve CRLF in templates for language-service (#33241)
This is a potential fix for https://github.com/angular/vscode-ng-language-service/issues/235
suggested by @andrius-pra in
47696136e3
.
Currently, CRLF line endings are converted to LFs and this causes the
diagnostics span to be off in templates that use CRLF. The line endings
must be preserved in order to maintain correct span offset. The solution
is to add an option to the Tokenizer to indicate such preservation.
PR Close #33241
This commit is contained in:
parent
e030375d9a
commit
65a0d2b53d
|
@ -101,6 +101,10 @@ export interface TokenizeOptions {
|
||||||
* included in source-map segments. A common example is whitespace.
|
* included in source-map segments. A common example is whitespace.
|
||||||
*/
|
*/
|
||||||
leadingTriviaChars?: string[];
|
leadingTriviaChars?: string[];
|
||||||
|
/**
|
||||||
|
* If true, do not convert CRLF to LF.
|
||||||
|
*/
|
||||||
|
preserveLineEndings?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function tokenize(
|
export function tokenize(
|
||||||
|
@ -134,6 +138,7 @@ class _Tokenizer {
|
||||||
private _currentTokenType: TokenType|null = null;
|
private _currentTokenType: TokenType|null = null;
|
||||||
private _expansionCaseStack: TokenType[] = [];
|
private _expansionCaseStack: TokenType[] = [];
|
||||||
private _inInterpolation: boolean = false;
|
private _inInterpolation: boolean = false;
|
||||||
|
private readonly _preserveLineEndings: boolean;
|
||||||
tokens: Token[] = [];
|
tokens: Token[] = [];
|
||||||
errors: TokenError[] = [];
|
errors: TokenError[] = [];
|
||||||
|
|
||||||
|
@ -153,6 +158,7 @@ class _Tokenizer {
|
||||||
options.range || {endPos: _file.content.length, startPos: 0, startLine: 0, startCol: 0};
|
options.range || {endPos: _file.content.length, startPos: 0, startLine: 0, startCol: 0};
|
||||||
this._cursor = options.escapedString ? new EscapedCharacterCursor(_file, range) :
|
this._cursor = options.escapedString ? new EscapedCharacterCursor(_file, range) :
|
||||||
new PlainCharacterCursor(_file, range);
|
new PlainCharacterCursor(_file, range);
|
||||||
|
this._preserveLineEndings = options.preserveLineEndings || false;
|
||||||
try {
|
try {
|
||||||
this._cursor.init();
|
this._cursor.init();
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
@ -161,6 +167,9 @@ class _Tokenizer {
|
||||||
}
|
}
|
||||||
|
|
||||||
private _processCarriageReturns(content: string): string {
|
private _processCarriageReturns(content: string): string {
|
||||||
|
if (this._preserveLineEndings) {
|
||||||
|
return content;
|
||||||
|
}
|
||||||
// http://www.w3.org/TR/html5/syntax.html#preprocessing-the-input-stream
|
// http://www.w3.org/TR/html5/syntax.html#preprocessing-the-input-stream
|
||||||
// In order to keep the original position in the source, we can not
|
// In order to keep the original position in the source, we can not
|
||||||
// pre-process it.
|
// pre-process it.
|
||||||
|
|
|
@ -510,6 +510,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
|
||||||
);
|
);
|
||||||
const htmlResult = htmlParser.parse(template.source, fileName, {
|
const htmlResult = htmlParser.parse(template.source, fileName, {
|
||||||
tokenizeExpansionForms: true,
|
tokenizeExpansionForms: true,
|
||||||
|
preserveLineEndings: true, // do not convert CRLF to LF
|
||||||
});
|
});
|
||||||
const {directives, pipes, schemas} = this.getModuleMetadataForDirective(classSymbol);
|
const {directives, pipes, schemas} = this.getModuleMetadataForDirective(classSymbol);
|
||||||
const parseResult =
|
const parseResult =
|
||||||
|
|
|
@ -645,30 +645,27 @@ describe('diagnostics', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// https://github.com/angular/vscode-ng-language-service/issues/235
|
|
||||||
// There is no easy fix for this issue currently due to the way template
|
|
||||||
// tokenization is done. In the example below, the whole string
|
|
||||||
// `\r\n{{line0}}\r\n{{line1}}\r\n{{line2}}` is tokenized as a whole, and then
|
|
||||||
// CR characters are stripped from it. Source span information is lost in the
|
|
||||||
// process. For more discussion, see the link above.
|
|
||||||
/*
|
|
||||||
it('should work correctly with CRLF endings', () => {
|
it('should work correctly with CRLF endings', () => {
|
||||||
|
// https://github.com/angular/vscode-ng-language-service/issues/235
|
||||||
|
// In the example below, the string
|
||||||
|
// `\r\n{{line0}}\r\n{{line1}}\r\n{{line2}}` is tokenized as a whole,
|
||||||
|
// and then CRLF characters are converted to LF.
|
||||||
|
// Source span information is lost in the process.
|
||||||
const fileName = '/app/test.ng';
|
const fileName = '/app/test.ng';
|
||||||
const content = mockHost.override(fileName,
|
const content =
|
||||||
'\r\n<div>\r\n{{line0}}\r\n{{line1}}\r\n{{line2}}\r\n</div>');
|
mockHost.override(fileName, '\r\n<div>\r\n{{line0}}\r\n{{line1}}\r\n{{line2}}\r\n</div>');
|
||||||
const ngDiags = ngLS.getDiagnostics(fileName);
|
const ngDiags = ngLS.getDiagnostics(fileName);
|
||||||
expect(ngDiags.length).toBe(3);
|
expect(ngDiags.length).toBe(3);
|
||||||
for (let i = 0; i < 3; ++i) {
|
for (let i = 0; i < 3; ++i) {
|
||||||
const {messageText, start, length} = ngDiags[i];
|
const {messageText, start, length} = ngDiags[i];
|
||||||
expect(messageText)
|
expect(messageText)
|
||||||
.toBe(
|
.toBe(
|
||||||
`Identifier 'line${i}' is not defined. The component declaration, template variable
|
`Identifier 'line${i}' is not defined. ` +
|
||||||
declarations, and element references do not contain such a member`);
|
`The component declaration, template variable declarations, and ` +
|
||||||
|
`element references do not contain such a member`);
|
||||||
// Assert that the span is actually highlight the bounded text. The span
|
// Assert that the span is actually highlight the bounded text. The span
|
||||||
// would be off if CRLF endings are not handled properly.
|
// would be off if CRLF endings are not handled properly.
|
||||||
expect(content.substring(start !, start ! + length !)).toBe(`line${i}`);
|
expect(content.substring(start !, start ! + length !)).toBe(`line${i}`);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
*/
|
|
||||||
|
|
||||||
});
|
});
|
||||||
|
|
Loading…
Reference in New Issue