From 734493c409b80450fa9a8725510fe0053a80801d Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Sat, 21 Sep 2024 00:24:54 -0700 Subject: [PATCH 1/2] Only look at parent when line and cursor indents match Given steps: - task: PowerShell@2 | We should be completing properties for the object that `task` is a part of, not the list under `steps`. Previously, the parent algorithm would see that the current indentation == the configured indentation and incorrectly choose the parent list as the "real" closest node, despite the cursor clearly being more indented than the list. Now, we also verify that the indentation of the current node equals the cursor indentation before deciding to look at the parent. --- src/languageservice/parser/yaml-documents.ts | 7 ++++- test/autoCompletion.test.ts | 27 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/languageservice/parser/yaml-documents.ts b/src/languageservice/parser/yaml-documents.ts index 3fb99ddd..e4abc7bb 100644 --- a/src/languageservice/parser/yaml-documents.ts +++ b/src/languageservice/parser/yaml-documents.ts @@ -197,7 +197,12 @@ export class SingleYAMLDocument extends JSONDocument { const position = textBuffer.getPosition(node.range[0]); const lineContent = textBuffer.getLineContent(position.line); currentLine = currentLine === '' ? lineContent.trim() : currentLine; - if (currentLine.startsWith('-') && indentation === configuredIndentation && currentLine === lineContent.trim()) { + if ( + currentLine.startsWith('-') && + indentation === configuredIndentation && + indentation === getIndentation(lineContent, position.character) && + currentLine === lineContent.trim() + ) { position.character += indentation; } if (position.character > indentation && position.character > 0) { diff --git a/test/autoCompletion.test.ts b/test/autoCompletion.test.ts index eba8fa6d..1517f710 100644 --- a/test/autoCompletion.test.ts +++ b/test/autoCompletion.test.ts @@ -2251,6 +2251,33 @@ describe('Auto Completion Tests', () => { expect(completion.items[0].insertText).eq('prop2: '); }); + it('should complete properties of an object under a list', async () => { + schemaProvider.addSchema(SCHEMA_ID, { + type: 'object', + properties: { + steps: { + type: 'array', + items: { + type: 'object', + properties: { + task: { + type: 'string', + }, + inputs: { + type: 'string', + }, + }, + }, + }, + }, + }); + + const content = 'steps:\n- task: PowerShell@2\n '; // len: 30 + const completion = await parseSetup(content, 30); + expect(completion.items).lengthOf(1); + expect(completion.items[0].label).eq('inputs'); + }); + it('should complete string which contains number in default value', async () => { schemaProvider.addSchema(SCHEMA_ID, { type: 'object', From 412c398a24cfd6532a2b2eb25db7b552cd03fccf Mon Sep 17 00:00:00 2001 From: Winston Liu Date: Thu, 31 Oct 2024 23:20:37 -0700 Subject: [PATCH 2/2] Do not complete object properties when cursor is directly under array Given steps: - task: PowerShell@2 | We can either complete schemas that are siblings of `steps` or continue adding onto the array, but what we _cannot_ do is suggest array items without the leading array indicator. In essence, this reverts #804 as my previous commit addresses that in a different way. --- .../services/yamlCompletion.ts | 18 +-------- test/autoCompletion.test.ts | 38 ++++++++++++++++++- test/autoCompletionFix.test.ts | 8 ++-- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/languageservice/services/yamlCompletion.ts b/src/languageservice/services/yamlCompletion.ts index 513a23d6..de159b07 100644 --- a/src/languageservice/services/yamlCompletion.ts +++ b/src/languageservice/services/yamlCompletion.ts @@ -477,20 +477,6 @@ export class YamlCompletion { node = pair.value; } } - } else if (isSeq(node)) { - if (lineContent.charAt(position.character - 1) !== '-') { - const map = this.createTempObjNode(currentWord, node, currentDoc); - map.items = []; - currentDoc.updateFromInternalDocument(); - for (const pair of node.items) { - if (isMap(pair)) { - pair.items.forEach((value) => { - map.items.push(value); - }); - } - } - node = map; - } } } } @@ -692,9 +678,7 @@ export class YamlCompletion { } for (const schema of matchingSchemas) { if ( - ((schema.node.internalNode === node && !matchOriginal) || - (schema.node.internalNode === originalNode && !hasColon) || - (schema.node.parent?.internalNode === originalNode && !hasColon)) && + ((schema.node.internalNode === node && !matchOriginal) || (schema.node.internalNode === originalNode && !hasColon)) && !schema.inverted ) { this.collectDefaultSnippets(schema.schema, separatorAfter, collector, { diff --git a/test/autoCompletion.test.ts b/test/autoCompletion.test.ts index 1517f710..6be38564 100644 --- a/test/autoCompletion.test.ts +++ b/test/autoCompletion.test.ts @@ -2251,7 +2251,7 @@ describe('Auto Completion Tests', () => { expect(completion.items[0].insertText).eq('prop2: '); }); - it('should complete properties of an object under a list', async () => { + it('should complete properties of an object under an array', async () => { schemaProvider.addSchema(SCHEMA_ID, { type: 'object', properties: { @@ -2269,15 +2269,51 @@ describe('Auto Completion Tests', () => { }, }, }, + name: { + type: 'string', + }, }, }); + // Thanks to the indentation, the intent is clear: continue adding to the current object. const content = 'steps:\n- task: PowerShell@2\n '; // len: 30 const completion = await parseSetup(content, 30); expect(completion.items).lengthOf(1); expect(completion.items[0].label).eq('inputs'); }); + it('should not show bare property completions for array items when the cursor indentation is ambiguous', async () => { + schemaProvider.addSchema(SCHEMA_ID, { + type: 'object', + properties: { + steps: { + type: 'array', + items: { + type: 'object', + properties: { + task: { + type: 'string', + }, + inputs: { + type: 'string', + }, + }, + }, + }, + name: { + type: 'string', + }, + }, + }); + + // Here, the intent _could_ be to move out of the array, or it could be to continue adding to the array. + // Thus, `name: ` is fine and so is `- task: `, but _not_ a bare `task: `. + const content = 'steps:\n- task: PowerShell@2\n'; // len: 28 + const completion = await parseSetup(content, 28); + expect(completion.items[0].label).eq('name'); + expect(completion.items.slice(1).filter((item) => !item.insertText.startsWith('- '))).to.be.empty; + }); + it('should complete string which contains number in default value', async () => { schemaProvider.addSchema(SCHEMA_ID, { type: 'object', diff --git a/test/autoCompletionFix.test.ts b/test/autoCompletionFix.test.ts index c720ce76..28cced44 100644 --- a/test/autoCompletionFix.test.ts +++ b/test/autoCompletionFix.test.ts @@ -129,16 +129,16 @@ describe('Auto Completion Fix Tests', () => { }, }, }); - const content = '- prop1: a\n | |'; // len: 12, pos: 11 - const completion = await parseCaret(content); + const content = '- prop1: a\n '; // len: 13 + const completion = await parseSetup(content, 1, 2); expect(completion.items).lengthOf(2); expect(completion.items[0]).eql( - createExpectedCompletion('prop2', 'prop2: ', 1, 3, 1, 4, 10, 2, { + createExpectedCompletion('prop2', 'prop2: ', 1, 2, 1, 2, 10, 2, { documentation: '', }) ); expect(completion.items[1]).eql( - createExpectedCompletion('prop3', 'prop3: ', 1, 3, 1, 4, 10, 2, { + createExpectedCompletion('prop3', 'prop3: ', 1, 2, 1, 2, 10, 2, { documentation: '', }) );