Skip to content
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

Merged

Conversation

cdleary
Copy link
Contributor

@cdleary cdleary commented Sep 28, 2024

See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_rename

Result is WorkspaceEdit which prefers documentChanges which contain TextDocumentEdit[]

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.)

Copy link
Collaborator

@hzeller hzeller left a 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.


TextDocumentEdit:
textDocument: OptionalVersionedTextDocumentIdentifier
edits: object # TextEdit[]
Copy link
Collaborator

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

Copy link
Collaborator

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+:).

Copy link
Contributor Author

@cdleary cdleary Oct 2, 2024

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.

Copy link
Collaborator

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>
@hzeller hzeller merged commit 86ee9ba into chipsalliance:master Oct 16, 2024
34 checks passed
copybara-service bot pushed a commit to google/xls that referenced this pull request Oct 16, 2024
@hzeller
Copy link
Collaborator

hzeller commented Oct 16, 2024

Alright, updated dependency in XLS with google/xls@59c3390

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants