godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

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

Open Zireael07 opened 4 years ago

Zireael07 commented 4 years ago

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: https://github.com/godotengine/godot/issues/3163

Calinou commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

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

According to https://github.com/godotengine/godot-proposals/issues/899#issuecomment-1077809766, 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 :slightly_smiling_face:

snouk-z commented 1 year ago

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 commented 1 year ago

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

JustAnotherCodemonkey commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

this is one of the most needed things right now!

Calinou commented 11 months ago

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

daphMonk commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

(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 :slightly_smiling_face:

amarraff commented 11 months ago

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 peanutbanana. You use CTRL + R to rename peanut to _peanutbutter, but in turn, this also changes _peanutbanana into _peanut_butterbanana. 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 _peanutbanana somewhere else in your code, that will also change to _peanut_butterbanana. This means an added second step of renaming _peanut_butterbanana back to _peanutbanana.

Furthermore, if you had a variable named _peanut_butter_bananasouffle, this would get even more confusing and time-consuming, because renaming peanut into _peanutbutter would turn it into _peanut_butter_butter_bananasouffle, breaking its references. If we assume _peanut_butter_bananasouffle was written after peanut was renamed - changing _peanut_butterbanana back to _peanutbanana would now change _peanut_butter_bananasouffle into _peanut_bananasouffle, 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 commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 :slightly_smiling_face:

Maran23 commented 9 months ago

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 commented 2 weeks ago

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.