feat: quickFix for enum, const, property#900
feat: quickFix for enum, const, property#900msivasubramaniaan merged 5 commits intoredhat-developer:mainfrom
Conversation
| for (const diagnostic of diagnostics) { | ||
| if (diagnostic.code === 'flowMap' || diagnostic.code === 'flowSeq') { | ||
| const node = getNodeforDiagnostic(document, diagnostic); | ||
| const node = getNodeForDiagnostic(document, diagnostic); |
| endCharacter, | ||
| severity, | ||
| source, | ||
| code |
There was a problem hiding this comment.
just code added to parameters
| length: propertyNode.keyNode.length, | ||
| }, | ||
| severity: DiagnosticSeverity.Warning, | ||
| code: ErrorCode.PropertyExpected, |
There was a problem hiding this comment.
not sure if it wouldn't be better to unify data.properties to data.values for the incorrect property
1bffa52 to
d6718e4
Compare
|
Hello @msivasubramaniaan, can I ask you for a review, please? |
| Array.isArray((diagnostic.data as YamlDiagnosticData).values) | ||
| ) { | ||
| return (diagnostic.data as YamlDiagnosticData).values; | ||
| } else if ( |
There was a problem hiding this comment.
Is this else really needed?
There was a problem hiding this comment.
Do you mean quickFix for properties? or can you explain your point further?
- I think that property quickFix can be useful
- note that there are different property names in
diagnostic.dataobject (valuesvsproperties)
| for (const value of values) { | ||
| results.push( | ||
| CodeAction.create( | ||
| value, |
There was a problem hiding this comment.
This returns only the value which is not very descriptive of the operation. perhaps we can replace it with something like
| value, | |
| `Use value "${value}"`, |
There was a problem hiding this comment.
I don't have preferences here, I can remember that I tried a few combinations but I ended up with only {value}.
I was inspired by the English spelling checker extension (Code Spell Checker extension)
small disadvantage of Use value "${value}" is that this PR adds quickfix also for properties of the object (not value), so we need to display the text based on the error type
- but for sure it's not a problem
so please make a decision
gorkem
left a comment
There was a problem hiding this comment.
Only a couple of minor comments. It also needs to be rebased.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
d6718e4 to
77bf8bc
Compare
|
@gorkem any updates here, please? |
77bf8bc to
5baa96e
Compare
|
@p-spacek Please upstream the branch |
@msivasubramaniaan I merged the main into this PR, but there is some schema change that caused the test to fail: UPDATE: I pushed the test fix for this schema rename |
…com/rh-yaml-language-server into feat/quick-fix-enum-const-property
What does this PR do?
add quick fix code-action for enums, consts, properties
What issues does this PR fix or reference?
no ref
Is it tested? How?
tests