From 803d5c521f6820a3d890cfd4dcf285d533e16a7b Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 27 Nov 2023 16:40:05 +0800 Subject: [PATCH 1/3] Fix some typings --- src/languageservice/services/yamlSelectionRanges.ts | 5 +---- src/languageservice/yamlLanguageService.ts | 2 +- test/yamlSelectionRanges.test.ts | 10 ++++++---- tsconfig.json | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/languageservice/services/yamlSelectionRanges.ts b/src/languageservice/services/yamlSelectionRanges.ts index b361ba8d7..a09ad71b5 100644 --- a/src/languageservice/services/yamlSelectionRanges.ts +++ b/src/languageservice/services/yamlSelectionRanges.ts @@ -3,10 +3,7 @@ import { yamlDocumentsCache } from '../parser/yaml-documents'; import { TextDocument } from 'vscode-languageserver-textdocument'; import { ASTNode } from 'vscode-json-languageservice'; -export function getSelectionRanges(document: TextDocument, positions: Position[]): SelectionRange[] | undefined { - if (!document) { - return; - } +export function getSelectionRanges(document: TextDocument, positions: Position[]): SelectionRange[] { const doc = yamlDocumentsCache.getYamlDocument(document); return positions.map((position) => { const ranges = getRanges(position); diff --git a/src/languageservice/yamlLanguageService.ts b/src/languageservice/yamlLanguageService.ts index 539371d8b..877fde067 100644 --- a/src/languageservice/yamlLanguageService.ts +++ b/src/languageservice/yamlLanguageService.ts @@ -175,7 +175,7 @@ export interface LanguageService { deleteSchemaContent: (schemaDeletions: SchemaDeletions) => void; deleteSchemasWhole: (schemaDeletions: SchemaDeletionsAll) => void; getFoldingRanges: (document: TextDocument, context: FoldingRangesContext) => FoldingRange[] | null; - getSelectionRanges: (document: TextDocument, positions: Position[]) => SelectionRange[] | undefined; + getSelectionRanges: (document: TextDocument, positions: Position[]) => SelectionRange[]; getCodeAction: (document: TextDocument, params: CodeActionParams) => CodeAction[] | undefined; getCodeLens: (document: TextDocument) => PromiseLike | CodeLens[] | undefined; resolveCodeLens: (param: CodeLens) => PromiseLike | CodeLens; diff --git a/test/yamlSelectionRanges.test.ts b/test/yamlSelectionRanges.test.ts index 3ac78ab41..2a1718165 100644 --- a/test/yamlSelectionRanges.test.ts +++ b/test/yamlSelectionRanges.test.ts @@ -12,14 +12,16 @@ function isRangesEqual(range1: Range, range2: Range): boolean { ); } -function expectSelections(selectionRange: SelectionRange, ranges: Range[]): void { +function expectSelections(selectionRange: SelectionRange | undefined, ranges: Range[]): void { for (const range of ranges) { - expect(selectionRange.range).eql(range); + expect(selectionRange?.range).eql(range); + // Deduplicate ranges - while (selectionRange.parent && isRangesEqual(selectionRange.range, selectionRange.parent.range)) { + while (selectionRange?.parent && isRangesEqual(selectionRange.range, selectionRange.parent.range)) { selectionRange = selectionRange.parent; } - selectionRange = selectionRange.parent; + + selectionRange = selectionRange?.parent; } } diff --git a/tsconfig.json b/tsconfig.json index 5294b2662..fa23d249a 100755 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,7 +9,7 @@ "outDir": "./out/server", "sourceMap": true, "target": "es2020", - "allowSyntheticDefaultImports": true, + "allowSyntheticDefaultImports": true, "skipLibCheck": true }, "include": [ "src", "test" ], From 57758235a2bf158e8ec6589ba3e2f92236bdd5cd Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 27 Nov 2023 16:40:52 +0800 Subject: [PATCH 2/3] Improve a few special cases --- .../services/yamlSelectionRanges.ts | 108 ++++++++++++------ 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/src/languageservice/services/yamlSelectionRanges.ts b/src/languageservice/services/yamlSelectionRanges.ts index a09ad71b5..7fc720310 100644 --- a/src/languageservice/services/yamlSelectionRanges.ts +++ b/src/languageservice/services/yamlSelectionRanges.ts @@ -7,74 +7,67 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] const doc = yamlDocumentsCache.getYamlDocument(document); return positions.map((position) => { const ranges = getRanges(position); - let current: SelectionRange; + let current: SelectionRange | undefined; for (const range of ranges) { current = SelectionRange.create(range, current); } - if (!current) { - current = SelectionRange.create({ - start: position, - end: position, - }); - } - return current; + return current ?? SelectionRange.create({ start: position, end: position }); }); function getRanges(position: Position): Range[] { const offset = document.offsetAt(position); const result: Range[] = []; for (const ymlDoc of doc.documents) { - let currentNode: ASTNode; - let firstNodeOffset: number; - let isFirstNode = true; + let currentNode: ASTNode | undefined; + let overrideStartOffset: number | undefined; ymlDoc.visit((node) => { const endOffset = node.offset + node.length; // Skip if end offset doesn't even reach cursor position if (endOffset < offset) { return true; } - let startOffset = node.offset; - // Recheck start offset with the trimmed one in case of this - // key: - // - value - // ↑ - if (startOffset > offset) { - const nodePosition = document.positionAt(startOffset); - if (nodePosition.line !== position.line) { - return true; - } - const lineBeginning = { line: nodePosition.line, character: 0 }; - const text = document.getText({ - start: lineBeginning, - end: nodePosition, - }); - if (text.trim().length !== 0) { + // Skip if we're ending at new line + // times: + // - second: 1 + // millisecond: 10 + // | - second: 2 + // ↑ millisecond: 0 + // (| is actually part of { second: 1, millisecond: 10 }) + // \r\n doesn't matter here + if (getTextFromOffsets(endOffset - 1, endOffset) === '\n') { + if (endOffset - 1 < offset) { return true; } - startOffset = document.offsetAt(lineBeginning); - if (startOffset > offset) { + } + + let startOffset = node.offset; + if (startOffset > offset) { + // Recheck start offset for some special cases + const newOffset = getStartOffsetForSpecialCases(node, position); + if (!newOffset || newOffset > offset) { return true; } + startOffset = newOffset; } // Allow equal for children to override if (!currentNode || startOffset >= currentNode.offset) { currentNode = node; - firstNodeOffset = startOffset; + overrideStartOffset = startOffset; } return true; }); while (currentNode) { - const startOffset = isFirstNode ? firstNodeOffset : currentNode.offset; + const startOffset = overrideStartOffset ?? currentNode.offset; const endOffset = currentNode.offset + currentNode.length; const range = { start: document.positionAt(startOffset), end: document.positionAt(endOffset), }; const text = document.getText(range); - const trimmedText = text.trimEnd(); - const trimmedLength = text.length - trimmedText.length; - if (trimmedLength > 0) { - range.end = document.positionAt(endOffset - trimmedLength); + const trimmedText = trimEndNewLine(text); + const trimmedEndOffset = startOffset + trimmedText.length; + if (trimmedEndOffset >= offset) { + range.end = document.positionAt(trimmedEndOffset); } // Add a jump between '' "" {} [] const isSurroundedBy = (startCharacter: string, endCharacter?: string): boolean => { @@ -92,7 +85,7 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] } result.push(range); currentNode = currentNode.parent; - isFirstNode = false; + overrideStartOffset = undefined; } // A position can't be in multiple documents if (result.length > 0) { @@ -101,4 +94,47 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] } return result.reverse(); } + + function getStartOffsetForSpecialCases(node: ASTNode, position: Position): number | undefined { + const nodeStartPosition = document.positionAt(node.offset); + if (nodeStartPosition.line !== position.line) { + return; + } + + if (node.parent?.type === 'array') { + // array: + // - value + // ↑ + if (getTextFromOffsets(node.offset - 2, node.offset) === '- ') { + return node.offset - 2; + } + } + + if (node.type === 'array' || node.type === 'object') { + // array: + // - value + // ↑ + const lineBeginning = { line: nodeStartPosition.line, character: 0 }; + const text = document.getText({ start: lineBeginning, end: nodeStartPosition }); + if (text.trim().length === 0) { + return document.offsetAt(lineBeginning); + } + } + } + + function getTextFromOffsets(startOffset: number, endOffset: number): string { + const start = document.positionAt(startOffset); + const end = document.positionAt(endOffset); + return document.getText({ start, end }); + } +} + +function trimEndNewLine(str: string): string { + if (str.endsWith('\r\n')) { + return str.substring(0, str.length - 2); + } + if (str.endsWith('\n')) { + return str.substring(0, str.length - 1); + } + return str; } From ea747c26dd64de3bc6d646787194595ccbc81f18 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 27 Nov 2023 17:47:59 +0800 Subject: [PATCH 3/3] Add tests --- .../services/yamlSelectionRanges.ts | 8 ++- test/yamlSelectionRanges.test.ts | 72 +++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/languageservice/services/yamlSelectionRanges.ts b/src/languageservice/services/yamlSelectionRanges.ts index 7fc720310..f60dcc91a 100644 --- a/src/languageservice/services/yamlSelectionRanges.ts +++ b/src/languageservice/services/yamlSelectionRanges.ts @@ -49,6 +49,7 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] } startOffset = newOffset; } + // Allow equal for children to override if (!currentNode || startOffset >= currentNode.offset) { currentNode = node; @@ -123,9 +124,10 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] } function getTextFromOffsets(startOffset: number, endOffset: number): string { - const start = document.positionAt(startOffset); - const end = document.positionAt(endOffset); - return document.getText({ start, end }); + return document.getText({ + start: document.positionAt(startOffset), + end: document.positionAt(endOffset), + }); } } diff --git a/test/yamlSelectionRanges.test.ts b/test/yamlSelectionRanges.test.ts index 2a1718165..93fd67e78 100644 --- a/test/yamlSelectionRanges.test.ts +++ b/test/yamlSelectionRanges.test.ts @@ -64,6 +64,20 @@ key: { start: { line: 1, character: 0 }, end: { line: 3, character: 8 } }, ]); + positions = [ + { + line: 3, + character: 3, + }, + ]; + ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 3, character: 2 }, end: { line: 3, character: 8 } }, + { start: { line: 2, character: 2 }, end: { line: 3, character: 8 } }, + { start: { line: 1, character: 0 }, end: { line: 3, character: 8 } }, + ]); + positions = [ { line: 2, @@ -78,6 +92,64 @@ key: ]); }); + it('selection ranges for array of objects', () => { + const yaml = ` +times: + - second: 1 + millisecond: 10 + - second: 2 + millisecond: 0 + `; + let positions: Position[] = [ + { + line: 4, + character: 0, + }, + ]; + const document = setupTextDocument(yaml); + let ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 2, character: 2 }, end: { line: 5, character: 18 } }, + { start: { line: 1, character: 0 }, end: { line: 5, character: 18 } }, + ]); + + positions = [ + { + line: 5, + character: 2, + }, + ]; + ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 4, character: 4 }, end: { line: 5, character: 18 } }, + { start: { line: 2, character: 2 }, end: { line: 5, character: 18 } }, + { start: { line: 1, character: 0 }, end: { line: 5, character: 18 } }, + ]); + }); + + it('selection ranges for trailing spaces', () => { + const yaml = ` +key: + - 1 + - 2 \t + `; + const positions: Position[] = [ + { + line: 2, + character: 9, + }, + ]; + const document = setupTextDocument(yaml); + const ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 2, character: 2 }, end: { line: 3, character: 9 } }, + { start: { line: 1, character: 0 }, end: { line: 3, character: 9 } }, + ]); + }); + it('selection ranges jump for "" \'\'', () => { const yaml = ` - "word"