-
Notifications
You must be signed in to change notification settings - Fork 277
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.
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
feat: property snippets #1041
base: main
Are you sure you want to change the base?
feat: property snippets #1041
Conversation
…-yaml-language-server into fix/suggest-hyphen-in-arrays
…y' into feat/property-snippets
… yaml - because it's properties from the parent object
private getInsertTextForArray(schema: any, separatorAfter: string, insertIndex = 1, indent = this.indentation): InsertText { | ||
private getInsertTextForArray( | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
schema: any, |
Check warning
Code scanning / ESLint
Disallow the `any` type Warning
return `${resultText}\n${indent}- ${ | ||
this.getInsertTextForArray(propertySchema.items, separatorAfter, 1, indent).insertText | ||
}`; | ||
let insertText = this.getInsertTextForArray(propertySchema.items, separatorAfter, collector, 1, indent).insertText; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix a few existing issues with snippet indentation
indent = this.indentation, | ||
insertIndex = 1 | ||
): InsertText { | ||
let insertText = ''; | ||
if (Array.isArray(schema.defaultSnippets) && schema.defaultSnippets.length === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main purpose of this PR is to add snippets to the object
} | ||
arrayTemplate = arrayInsertLines.join('\n'); | ||
} | ||
const arrayInsertResult = this.getInsertTextForArray( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor and fix indentation
use new addIndentationToMultilineString
function
@@ -1491,6 +1540,24 @@ export class YamlCompletion { | |||
if (insertText === '' && value) { | |||
continue; | |||
} | |||
|
|||
if ((arrayDepth === 0 && type === 'array') || isArray) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor, uses a new collector.context concept to distinguish between different scenarios
It wasn't possible to detect if a hyphen was missing or if the cursor was after the colon in the schema array
The previous implementation knows only about schema and a few input parameters (add a new line, add indent, array Depth), but it's useful to know what the current line looks like
@@ -37,7 +37,7 @@ export function stringifyObject( | |||
let result = ''; | |||
for (let i = 0; i < obj.length; i++) { | |||
let pseudoObj = obj[i]; | |||
if (typeof obj[i] !== 'object') { | |||
if (typeof obj[i] !== 'object' || obj[i] === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix the null issue when the snippet contains null as a value
it crashed when the snippet looks like this body: ["string", 5, null]
@@ -87,3 +87,18 @@ export function getFirstNonWhitespaceCharacterAfterOffset(str: string, offset: n | |||
} | |||
return offset; | |||
} | |||
|
|||
export function addIndentationToMultilineString(text: string, firstIndent: string, nextIndent: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unified function to indent multiline strings, this logic was written multiple times before.
@@ -63,6 +64,20 @@ interface CompletionsCollector { | |||
getNumberOfProposals(): number; | |||
result: CompletionList; | |||
proposed: { [key: string]: CompletionItem }; | |||
context: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The collector is sent to each internal function, so let's add some context data so we can distinguish between some specific yaml situation
What does this PR do?
The main purpose of this PR is to support property snippet intellisense
will insert the value from the snippet
A similar approach is used in different situations when the single snippet is used as a default value
but this PR also unifies (replace) related changes from PRs:
#755
#901
and include some other snippet fixes - examples are in new tests
so if you accept this PR, we can close 2 previous PRs
Let me know if I can add more info, please. It would be nice to include these changes in your repo.
We use snippets In our schemas a lot as a rich set of examples and sometimes as a replacement for autogenerated code structure. So, I believe that the code is well-tested.
What issues does this PR fix or reference?
no ref
Is it tested? How?
existing and a few new unit tests
tested by QA in our separate branch