X Tutup
Skip to content

Implement proper LSP file rename/delete (moves/deletes .uid/.import files)#105536

Open
Cretezy wants to merge 1 commit intogodotengine:masterfrom
Cretezy:push-tnryluzukrno
Open

Implement proper LSP file rename/delete (moves/deletes .uid/.import files)#105536
Cretezy wants to merge 1 commit intogodotengine:masterfrom
Cretezy:push-tnryluzukrno

Conversation

@Cretezy
Copy link
Contributor

@Cretezy Cretezy commented Apr 18, 2025 •
edited
Loading

Fixes #105515

Implement willRenameFiles, which provides to the LSP client a command to rename the matching .uid (for .gd files) or .import (for other files), if they exist.

Implement willDeleteFiles, which provides to the LSP client a command to delete the matching .uid (for .gd files) or .import (for other files), if they exist.

Tested working within Neovim with LSP.

@Cretezy Cretezy requested a review from a team as a code owner April 18, 2025 18:35
@Cretezy Cretezy mentioned this pull request Apr 18, 2025
Open
HolonProduction
HolonProduction reviewed Apr 18, 2025
View reviewed changes
Copy link
Member

@HolonProduction HolonProduction left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment everything we add to godot_lsp.h is additional maintenance burden, so if something isn't used by the LSP we really should not add it in there.

In addition I have no clue how you tested this and it worked, since the new endpoints are not bound to JSON rpc. Please make sure to rebase your changes on top of the most recent Godot master and test again.

@Cretezy
Copy link
Contributor Author

Cretezy commented Apr 18, 2025

@HolonProduction Thank you a ton for the review, I'm still learning the inner workings of LSP (and I'm not too familiar with C++).

I've updated the PR based on the changes you suggested. I previously was basing my branch off the 4.4 branch, hence why the SET_WORKSPACE_METHOD was missing, should've tested again after rebasing.

What do you suggest about the changes in godot_lsp.h? I could remove the changes there (the new structs, not the glob, as that's required) and manually build the Dictionary in the methods.

Here's it working on latest:

"rpc.send"	{ id = 4, jsonrpc = "2.0", method = "workspace/willRenameFiles", params = { files = { { newUri = "file:///home/charles/Games/Testing/godot-lsp/test3.gd", oldUri = "file:///home/charles/Games/Testing/godot-lsp/test2.gd" } } } }
"rpc.receive"	{ jsonrpc = "2.0", method = "workspace/willRenameFiles", params = { documentChanges = { { kind = "rename", newUri = "file:///home/charles/Games/Testing/godot-lsp/test3.gd.uid", oldUri = "file:///home/charles/Games/Testing/godot-lsp/test2.gd.uid" } } } }

@Cretezy Cretezy force-pushed the push-tnryluzukrno branch 2 times, most recently from 1dea80c to 639a4c7 Compare April 18, 2025 20:55
@AThousandShips AThousandShips added this to the 4.x milestone Apr 19, 2025
@Cretezy Cretezy requested a review from HolonProduction April 20, 2025 17:17
HolonProduction
HolonProduction reviewed Apr 20, 2025
View reviewed changes
HolonProduction
HolonProduction reviewed Apr 20, 2025
View reviewed changes
@Cretezy Cretezy force-pushed the push-tnryluzukrno branch 2 times, most recently from 34b90b7 to cbf46db Compare April 20, 2025 17:52
@Cretezy
Copy link
Contributor Author

Cretezy commented Apr 20, 2025 •
edited
Loading

Updated to reply instead:

"rpc.send"	{ id = 3, jsonrpc = "2.0", method = "workspace/willRenameFiles", params = { files = { { newUri = "file:///home/charles/Games/Testing/godot-lsp/test5.gd", oldUri = "file:///home/charles/Games/Testing/godot-lsp/test4.gd" } } } }
"rpc.receive"	{ id = 3, jsonrpc = "2.0", result = { documentChanges = { { kind = "rename", newUri = "file:///home/charles/Games/Testing/godot-lsp/test5.gd.uid", oldUri = "file:///home/charles/Games/Testing/godot-lsp/test4.gd.uid", options = { ignoreIfExists = true } } } } }

"rpc.send"	{ id = 5, jsonrpc = "2.0", method = "workspace/willDeleteFiles", params = { files = { { uri = "file:///home/charles/Games/Testing/godot-lsp/test5.gd" } } } }
"rpc.receive"	{ id = 5, jsonrpc = "2.0", result = { documentChanges = { { kind = "delete", options = { ignoreIfNotExists = true }, uri = "file:///home/charles/Games/Testing/godot-lsp/test5.gd.uid" } } } }

@Cretezy Cretezy force-pushed the push-tnryluzukrno branch from cbf46db to ff31e9b Compare April 20, 2025 18:09
@Cretezy Cretezy requested a review from HolonProduction April 20, 2025 18:09
HolonProduction
HolonProduction reviewed Apr 20, 2025
View reviewed changes
@Cretezy Cretezy force-pushed the push-tnryluzukrno branch 2 times, most recently from 2047fa4 to 72c9990 Compare April 20, 2025 18:18
@Cretezy Cretezy requested a review from HolonProduction April 20, 2025 18:19
@Cretezy Cretezy force-pushed the push-tnryluzukrno branch from 72c9990 to 89777f7 Compare April 20, 2025 18:27
@HolonProduction
Copy link
Member

HolonProduction commented Apr 26, 2025 •
edited
Loading

Doesn't work with VSCode. Reason seems to be the glob, maybe neovim is more permissive than the spec. But to match all files correctly you need to use ** (or maybe even **/*.* not sure if there could be problems with matching an empty path). Also you accidentally modified the existing glob for didDeleteFiles.

Also please remove the fixes reference from the commit message, it's enough to have it in the PR description. It creates a lot of noise on Github when in the commit.

@HolonProduction
Copy link
Member

HolonProduction commented Jun 27, 2025

So this has been sitting for a while now, @Cretezy are you still interested in working on it? Or should someone else pick up the work?

@HolonProduction
Copy link
Member

HolonProduction commented Aug 19, 2025

Marking as salvageable for now.

@Cretezy
Copy link
Contributor Author

Cretezy commented Aug 19, 2025

Hey, haven't been working on Godot recently. I'll try to push this over the line this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HolonProduction HolonProduction Awaiting requested review from HolonProduction

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

enhancement topic:editor topic:gdscript

Projects

None yet

Milestone

4.x

Development

Successfully merging this pull request may close these issues.

LSP file rename/delete doesn't rename/delete .uid file

3 participants

@Cretezy @HolonProduction @AThousandShips
X Tutup