godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Implement a code formatter for GDScript in the script editor #3630

Open Razoric480 opened 2 years ago

Razoric480 commented 2 years ago

Describe the project you are working on

I've been sponsored by GDQuest to make improvements to the godot engine.

Describe the problem or limitation you are having in your project

See original proposal: This supersedes #1200.

Maintaining a uniform code style between files is time-consuming, and when working with multiple people, it becomes nearly impossible. There is one third party option, https://github.com/Scony/godot-gdscript-toolkit by Scony, but it has to create a syntax tree before every parse - which takes time at boot-up, cannot be run from within Godot, and runs on Python and has to manually text-parse scripts.

If Godot offered the ability to format code, it could be done in-editor and fast (using the built-in parser with C++), and could be out-sourced to the Language Server with the DocumentFormatting request for outside editors.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The new GDScript parser for GDScript 2 does (almost) everything we need it to in order to make a formatter, except for comments and blank lines. Fulfilling this proposal involves:

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Screenshot_1

The formatter combines text-parsing to find comments and manual new lines, and the GDScript parser for actual code in order to re-create the script in the official style guide. It breaks up lines beyond 80 characters using the:

function(
        param1,
        param2
)

syntax and puts arrays/dictionaries/enums on their own lines when they cannot fit, and adds/removes trailing commas in those collections accordingly.

It then re-introduces comments and manual new lines, splits suites up, and otherwise does its utmost best to make it clean and readable while maintaining the coder's organization.

I already have some proof of concept coding working in a branch on my fork, though it's still early goings.

Screenshot_2

Consideration

It could potentially also re-organize code, grouping constants, signals, exports, vars, onready vars, etc together and putting them in style guide order. This may be more in line with re-factoring tools than pure code formatting, however, so may be best left for those.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Any other option, like Scony's toolkit, requires re-inventing the wheel and comes with other disadvantages, like performance and ease-of-use, not to mention it would be a very large and involved bit of scripting.

Is there a reason why this should be core and not an add-on in the asset library?

There's a perfectly good parser inside of Godot that we don't have access to outside of C++.

Calinou commented 2 years ago

Add a new option to godot's CLI specifically for batch code formatting. It would take a list of space delineated GDScript files in the project and clean them in series.

Godot has a --script CLI option can be combine with --check-only to check for syntax. Maybe we can combine --script with a --format option? This --script option doesn't support checking multiple scripts yet, but I suppose this could be added (e.g. by reading all positional arguments and checking them one by one).

It might make the (already messy) CLI parser even more messy, so going for a simpler option is perhaps better. I'm just unsure about using comma-separated values – it seems using space-separated values is more common (which means reading all the positional arguments as if they were script names). Space-separated values also allow for shell globbing:

# Format all scripts within the project.
godot --format **/*.gd

Also, CLI formatting should alter the process' exit code so it can be used in continuous integration scenarios:

NathanLovato commented 2 years ago

For the cli the --format option is great, and yes the values should be space-separated for globbing.

It could potentially also re-organize code, grouping constants, signals, exports, vars, onready vars, etc together and putting them in style guide order. This may be more in line with re-factoring tools than pure code formatting, however, so may be best left for those.

Such a refactoring tool would be great I think, for many scripts. The official style guide has a particular code order and that ordering can be automated reliably. However, it should definitely at least be a separate tool or menu option.

But I'm all for it: combined with @Calinou's suggestion to use exit codes, it'd allow to automatically check the code in official demos follows the standard style and code order very efficiently.

amatrelan commented 2 years ago

The official style guide has a particular code order and that ordering can be automated reliably. However, it should definitely at least be a separate tool or menu option.

I agree refactoring tools would be nice but I think if that kinda tools there should be configuration for things, this is subject what can be too intrusive if done wrong.

Also one more settings I would love to see in this feature would be

The reason I don't use those https://github.com/Scony/godot-gdscript-toolkit in formatting is that I cannot choose to use spaces and it overwrites those.

Calinou commented 2 years ago

Also one more settings I would love to see in this feature would be

Both of those settings are already available in the Editor Settings. The code formatter will make use of those :slightly_smiling_face:

Razoric480 commented 2 years ago

Yep image

amatrelan commented 2 years ago

Ah OK, don't use internal editor at all myself so never looked from there those settings for editor. Hmm would there possibility have mirror settings like if there would be same settings in Language Server section?

Because this would clarify for people who use only external editors with LSP.

Razoric480 commented 2 years ago

textDocument/formatting has an options parameter with tabSize and insertSpaces, so presumably it'd be configured on the extension/plugin that supports it. The language server function can then use those as overrides.

akien-mga commented 2 years ago

We discussed this in a proposal review meeting and confirmed that we like the idea, it would be great to have this in the next Godot version.

Current status is that this is partly implemented in https://github.com/godotengine/godot/pull/55835, we need @godotengine/gdscript @godotengine/script-editor folks to give it a try and provide some review feedback so @Razoric480 can finalize the implementation when time permits.

NathanLovato commented 2 years ago

Razoric's keeping at it slowly while waiting for review, but yeah we'd really need a review from the GDScript team.

chexmo commented 9 months ago

This project https://github.com/Scony/godot-gdscript-toolkit is alive and we also have and asset library entry to add it from inside Godot... but i believe it isn't enough!

I believe we should consider including it as a built-in feature ootb.

Haters like to talk about C# and GDScript, but GDScript is one of the key features of Godot, and the Godot's text editor kinda works very fine for it. The editor just needs some modern tools like this formatter and maybe a member rename tool (i.e. to rename a variable across all the scripts or to rename a function across the project including signals and stuff)

(PS: idk if there is a feature request for this renamer)

Calinou commented 9 months ago

@chexmo Both the formatter and renaming functionality are planned to be implemented in core at some point :slightly_smiling_face:

We can't give an ETA on when these will be completed though.

m21-cerutti commented 4 months ago

Could be very usefull indeed, but it should not only be available in editor, cli would be even more important. It could be uesd for example with pre-commit hooks and would simplify the process when contributing to demos. https://github.com/godotengine/godot-demo-projects/issues/1090

Pheubel commented 6 days ago

I like the idea of having a proper formatter, but the thing that concerns me is it being described as settings for the editor, while this makes sense for format on save. in cases where you are working with two different projects that use a slight variation in formatting settings, this could get bothersome.

In other programming languages & projects it is not uncommon to have a separate file that dictates to the formatter what rules it should follow. I do see merit in having a formatter style be applied editor wide, so that it can be shared over different projects, so perhaps both options could exist, where if the project does not have a file with formatting rules, it could fall back on the editor's formatting rules. perhaps it could also be useful to generate a formatting rules file from the editor setting's rules.