Add rename support for YAML anchors and aliases #1149
Conversation
datho7561
left a comment
datho7561
left a comment
There was a problem hiding this comment.
Very helpful as is, but I think it could be better.
| } | ||
|
|
||
| doRename(document: TextDocument, params: RenameParams): WorkspaceEdit | null { | ||
| try { |
There was a problem hiding this comment.
Currently with this PR, it's possible to rename the anchor to a name that's invalid (for instance, add any of [, ], (, ), ,). For a list of what characters are valid for an anchor, here's what the specification says: https://yaml.org/spec/1.2.2/#rule-ns-anchor-char.
There's a way to reject these names. Are you interested in working on this? I think it's important to have, but it's a bit more work, and as it stands this PR is helpful as-is.
| return null; | ||
| } | ||
|
|
||
| const normalizedNewName = this.normalizeName(params.newName); |
There was a problem hiding this comment.
This will remove the first * or & from the supplied new name. It seems (based on the spec) that anchors are allowed to contain them, even at the beginning. Given just the name and not the leading & or * is being replaced, is this necessary?
| } | ||
| }); | ||
|
|
||
| if (edits.length === 0) { |
There was a problem hiding this comment.
Should never happen, see line 58
3 participants
What does this PR do?
What issues does this PR fix or reference?
Is it tested? How?
npm test -- --grep "YAML Rename"