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

Add "refactor" tooling to rename symbols in the script editor #899

Open
Zireael07 opened this issue May 26, 2020 · 28 comments
Open

Add "refactor" tooling to rename symbols in the script editor #899

Zireael07 opened this issue May 26, 2020 · 28 comments

Comments

@Zireael07
Copy link

Describe the project you are working on:
N/A (generic issue)
Describe the problem or limitation you are having in your project:
Renaming a variable in a script causes references to it in other script to be broken.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
When renaming a variable, it should refactor/rename references too.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Quoted from original issue:
When renaming any kind of object in Godot, it should find all occurrences in scripts/editor
and change that as well. This can be achieved by parsing script files, any matching names for the changed scene/node, like "load(), preload(), set_script() get_node()..etc!) functions.

This should apply too to renaming variables, it would be great if when renaming a variable in script
it changes all occurrences in the script file.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Nope. (Also I assume it will be used more often the bigger and more complex projects are)

Is there a reason why this should be core and not an add-on in the asset library?:
Has to be done in script editor for variables use case, and in the editor itself (possibly Fix Dependencies dialog) for other use case.

Original issue: godotengine/godot#3163

@Calinou
Copy link
Member

Calinou commented May 26, 2020

Note that this will probably require a dedicated "refactor" tool to be added, similar to the one in JetBrains IDEs or Visual Studio Code. Search and replace should generally not attempt to be too smart on its own as to not break users' expectations.

@Zireael07
Copy link
Author

Yep, I don't think it's a task for search-and-replace. (Also, in most cases, I change variable names manually and not via search)

@NathanLovato
Copy link

This is a feature the GDScript language server could cover, it's part of the specification iirc. Implementing it there would make it available in all editors like VSCode, Atom, Emacs...

@Calinou
Copy link
Member

Calinou commented Mar 24, 2022

As a stopgap solution, the Find in Files dialog could have checkboxes to only find and replace within strings, comments and code ("code" being anything that's not a comment or string). In many cases, this is sufficient to rename all instances of a function or variable within the codebase. Any of the 3 checkboxes could be enabled to make refactoring code easier. The upside of this approach is that it can be made to work easily on scripts of various languages (including C#).

Allowing the use of regular expressions for searching and replacing would make this even more powerful, although we'd have to implement a syntax for reusing capture groups in the replacement string (like VS Code's $1, $2, …).

JetBrains IDEs provide similar functionality.

@NathanLovato
Copy link

The rename feature is implemented in Godot for GDScript. It's accessible via the language server protocol, so right now only in an external editor, but the code to rename a symbol in the project is there. So perhaps it would be relatively quick to expose in the script editor now?

A proper symbol rename is really useful as you don't have to worry about filtering or checking your search results in case you use the same terms throughout your codebase.

Regex would also be useful, of course!

@thieme55
Copy link

The rename feature is implemented in Godot for GDScript. It's accessible via the language server protocol, so right now only in an external editor, but the code to rename a symbol in the project is there. So perhaps it would be relatively quick to expose in the script editor now?

I look into this because I'm very intrested in this functionality and it seems that the script editor does not yet use the language server.

I would like to help to bring refactoring tools to the editor, but I think it would only make sense to do it via the language server, to not dublicate gdscript refactoring logic.

Can someone tell me if this would be desired? @Calinou maybe?

@Calinou
Copy link
Member

Calinou commented Apr 15, 2022

I would like to help to bring refactoring tools to the editor, but I think it would only make sense to do it via the language server, to not dublicate gdscript refactoring logic.

Can someone tell me if this would be desired?

Making the Godot script editor a language client would imply a lot of work, which I feel adds too much complexity to be worth it. To prevent the codebase from getting too complex, Godot does not aim to be absolutely DRY in all aspects.

@ghostbutter-games
Copy link

Are there any plans yet towards adding refactoring / renaming of symbols for 4.0 or 4.1?
I believe I read somewhere that a relevant PR by Juan was merged for 4.0/master some months ago, which could in theory make it possible.

It would be extremely appreciated, especially when coupled with an external code editor.
Right now, renaming files or symbols outside the editor can blow up the entire project.

@Calinou
Copy link
Member

Calinou commented Oct 4, 2022

Are there any plans yet towards adding refactoring / renaming of symbols for 4.0 or 4.1?

According to #899 (comment), the tooling is already present in the GDScript LSP server (which is in the editor), but there is no GUI for using it within the editor itself.

However, we're in feature freeze now, so now isn't the time to work on large sweeping changes to the script editor. Therefore, this feature will land in 4.1 at the earliest. We have to focus on releasing 4.0 one day 🙂

@snouk-z
Copy link

snouk-z commented Nov 30, 2022

UX-wise, I made some mockups for this feature. They were planned to be a part of another proposal but since that one exists :

Option in the right-click menu
refactor_gds

Dialog for refactoring a function
refac_func_dialog

...and for a variable
refact_var

@Calinou
Copy link
Member

Calinou commented Nov 30, 2022

Ctrl + Shift + R is already used for the Replace in Files dialog, so another shortcut would have to be picked.

@JustAnotherCodemonkey
Copy link

Ok so now that we have Godot 4.1 out, is there anyone currently working on this? What's the status? This would be a really nice feature to have.

@Calinou
Copy link
Member

Calinou commented Sep 9, 2023

Ok so now that we have Godot 4.1 out, is there anyone currently working on this? What's the status? This would be a really nice feature to have.

To my knowledge, nobody is currently working on this. If my above comment is correct (I can't guarantee it is), it should be relatively easy to do as it only involves exposing the functionality in the built-in script editor.

@Ostreicher

This comment was marked as off-topic.

@Calinou
Copy link
Member

Calinou commented Sep 24, 2023

@Ostreicher Please don't bump issues without contributing significant new information. Use the 👍 reaction button on the first post instead.

@daphMonk
Copy link

daphMonk commented Oct 2, 2023

I'll be looking into this feature and see what's missing to expose it in the editor. I'm new to Godot (and I don't have a lot of free time) so I might not be super fast. Not sure if it's worth it assigning it to me just yet but I thought I should let you know.

@daphMonk
Copy link

daphMonk commented Oct 8, 2023

Hey, I've been working on it a bit and I almost got something. But I realized that the rename function mentionned above (found here GDScriptWorkspace::rename) returns a dictionary containing the changes, but it never modifies the files accordingly. I wasn't able to find a function whose role would be to use this return value to apply the changes. Wanted to make sure it didn't already exist before implementing it. If anyone knows, let me know! (And if there's a more appropriate space where I should be asking these technical questions, let me know as well.)

@Calinou
Copy link
Member

Calinou commented Oct 8, 2023

(And if there's a more appropriate space where I should be asking these technical questions, let me know as well.)

I suggest joining the Godot Contributors Chat's #gdscript channel 🙂

@amarraff
Copy link

amarraff commented Oct 12, 2023

A key point in favor of this feature that I didn't see in this thread: it would solve the problem of renaming the wrong variables. I think it's crucial that the feature be added for this reason. Here's an example of what I mean:

You have two vars, var peanut and var peanut_banana. You use CTRL + R to rename peanut to peanut_butter, but in turn, this also changes peanut_banana into peanut_butter_banana. This will likely not be a desired name change for the affected variable. This will also rename any instance of the word "peanut" in comments, since it's generic and just looks for the string, so if you have a comment describing peanut_banana somewhere else in your code, that will also change to peanut_butter_banana. This means an added second step of renaming peanut_butter_banana back to peanut_banana.

Furthermore, if you had a variable named peanut_butter_banana_souffle, this would get even more confusing and time-consuming, because renaming peanut into peanut_butter would turn it into peanut_butter_butter_banana_souffle, breaking its references. If we assume peanut_butter_banana_souffle was written after peanut was renamed - changing peanut_butter_banana back to peanut_banana would now change peanut_butter_banana_souffle into peanut_banana_souffle, and break its references in other scripts as well. It can cause some real headaches if you have a group of similarly named variables, which are referenced across your project, and you only want to rename one of them. Your only option is often to do it manually, in each instance where it is written.

@daphMonk
Copy link

Hi, I have something good enough to show and ask for feedback. I want to know if this is how people envision it.

It is quite simple for now and I wonder if this is enough or if people would prefer it to look like Replace in Files.
Rename : https://ttprivatenew.s3.amazonaws.com/pulse/daphn/attachments/22232005/RenameFirstPass.mp4

Replace in files : https://daphn.tinytake.com/media/1533bc8?filename=1697318908837_Replace1.png&sub_type=thumbnail_preview&type=attachment&width=677&height=323
https://daphn.tinytake.com/media/1533bcb?filename=1697319011807_Replace2.png&sub_type=thumbnail_preview&type=attachment&width=1195&height=199

@yulrun
Copy link

yulrun commented Nov 18, 2023

Hi, I have something good enough to show and ask for feedback. I want to know if this is how people envision it.

It is quite simple for now and I wonder if this is enough or if people would prefer it to look like Replace in Files. Rename : https://ttprivatenew.s3.amazonaws.com/pulse/daphn/attachments/22232005/RenameFirstPass.mp4

Replace in files : https://daphn.tinytake.com/media/1533bc8?filename=1697318908837_Replace1.png&sub_type=thumbnail_preview&type=attachment&width=677&height=323 https://daphn.tinytake.com/media/1533bcb?filename=1697319011807_Replace2.png&sub_type=thumbnail_preview&type=attachment&width=1195&height=199

That’s looking Great! Can’t wait to hear more about the progress.

@Ostreicher
Copy link

Ostreicher commented Nov 19, 2023

thats amazing man !! now the icing on the cake would be shortcut f2, autofocussing the rename field, confirming with return and auto reload afterwards haha. but looks already awesome 👍

@daphMonk
Copy link

daphMonk commented Nov 19, 2023

thats amazing man !! now the icing on the cake would be shortcut f2, autofocussing the rename field, confirming with return and auto reload afterwards haha. but looks already awesome 👍

Autofocusing on text is there, as well as confirming with return, and auto reload works as long as you have the setting activated in the Editor Settings. Is that what you had in mind for auto reload?

I'll be tidying up the code a bit. And I might consider sending it for review after.

@yulrun
Copy link

yulrun commented Nov 19, 2023

Autofocusing on text is there, as well as confirming with return, and auto reload works as long as you have the setting activated in the Editor Settings. Is that what you had in mind for auto reload?

I'll be tidying up the code a bit. And I might consider sending it for review after.

Hope to see it added internally or as an addon in the near future. Be nice to not have to do all my refactoring in VSCode right now and stay in the editor for everything.

Thank you for the effort you put in.

@Calinou
Copy link
Member

Calinou commented Nov 20, 2023

Please remove nested quote levels when quoting a post, as it ends up taking too much space over time.

I've edited posts accordingly, but remember to do this in the future 🙂

@Maran23
Copy link

Maran23 commented Dec 13, 2023

I'll be tidying up the code a bit. And I might consider sending it for review after.

Note that you can open a PR and mark it as draft if you want to gather early feedback from other people🙂

@domske
Copy link

domske commented Sep 1, 2024

This would be a great feature. Like F2 in VS Code. I use CTRL + D to select all same occurrences and then edit with the multi-cursor. Ok, this does not always work, if you have similar names with other suffix/prefixes. Anyway...

A global refactoring including node names etc. may not be trivial to implement. But I would be happy if we only had that for the currently open file I editing. Hit F2 and rename the variable on the current cursor position. Just like in VS Code. This should work in gdscript and shaders, etc.

@reidhannaford
Copy link

And I might consider sending it for review after.

@daphMonk was this ever submitted for review? And out of curiosity: in your implementation does renaming a symbol apply to your whole project or just that one script?

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

No branches or pull requests