-
Notifications
You must be signed in to change notification settings - Fork 214
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
[LSP] Add TextDocumentEdit for textDocument/rename support. #2262
[LSP] Add TextDocumentEdit for textDocument/rename support. #2262
Conversation
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.
LGTM, but wanted to check if you maybe want a typed vector for the edits.
Of course, it would then loose the flexibility of one-of (TextEdit | AnnotatedTextEdit)
, but not sure if that is planned.
common/lsp/lsp-protocol.yaml
Outdated
|
||
TextDocumentEdit: | ||
textDocument: OptionalVersionedTextDocumentIdentifier | ||
edits: object # TextEdit[] |
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.
If you need an array of things and don't want it untyped, you can mark it with a plus to get a vector of TextEdit
.
edits+: TextEdit
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.
LMK if you want to use the object
version or change to TextEdit[]
(with edits+:
).
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.
No it sounds like a good idea and I hadn't realized the edits+ trick, sorry for the delay, should be able to push a new rev on Thurs.
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.
What is the status here ?
Via github UI Co-authored-by: Henner Zeller <h.zeller@acm.org>
By @cdleary in this PR chipsalliance/verible#2262 PiperOrigin-RevId: 686557188
Alright, updated dependency in XLS with google/xls@59c3390 |
See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_rename
Result is
WorkspaceEdit
which prefersdocumentChanges
which containTextDocumentEdit[]
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspaceEdit
(Note: we can do this with support for
changes?
in the LSP but this is the suggested way for new implementations, so figured I'd add.)