Prototype a GDExtension for the formatter

[khellste]

(Revised based on discussion in this thread)

We can use godot-rust to expose the formatter as a library to @tools in the Godot editor. This would be an improvement for developers who want to focus on the native Godot editor experience and want a plug-and-play addon that doesn't need an internet connection to download additional dependencies on first use. It would probably also provide a slight performance improvement for Godot editor users, because the addon would not need to shell out to a separate process to format scripts.

Maintainer edit: This also means when people update the extension, they will automatically get the latest version of the formatter and they won't need to update the add-on and the formatter separately, because this will compile the formatter library into the extension.

Previous proposal

Now that the project no longer depends on Topiary, there is less reason for the formatter to be implemented as an external binary.

I propose a full rewrite in GDExtension/C++, which would have the following advantages:

• The formatter can be distributed as a single Godot addon. No need to download and install a separate binary.
• It runs natively in the Godot editor without shelling out to a separate process. (Though we should still support external editors using godot --headless -s some_cli_wrapper.gd.)
• Formatter primitives (like perhaps the GDScript AST) can be exposed to @tools in the Godot scripting environment, allowing users to more easily build custom tooling on top of the formatter.

I realize it's a big undertaking, but I think having the formatter as a standalone Godot addon would be worth it. I'm willing to take a stab at it if others on the project see value in it too.

[R2robot]

Would it still be usable from external editors?

[khellste]

Would it still be usable from external editors?

It should be doable using godot --headless -s some_new_cli_wrapper_script.gd -- arg1 arg2 arg3, where ideally the wrapper script is just a thin wrapper that marshalls the command line arguments into some formatter options and then calls into the formatter GDExtension. (Added a mention of this to the original issue.)

[R2robot]

It should be doable using godot --headless -s some_new_cli_wrapper_script.gd -- arg1 arg2 arg3

I've never run anything through godot --headless before. Does that launch a new instance of Godot, or will it use the currently running instance? If it runs a new one, it sounds like it would be a bit heavy to run for my current config which formats on save.... and I save a lot.

[khellste]

Yes, headless mode launches a new instance of Godot, which does sound slower. So you make a good point.

Maybe for the CLI we would release a standalone C++ binary (likely as fast as Rust, maybe faster), which could share almost all its code with the GDExtension.

[NathanLovato]

Some feedback on this. In short:

• Compiling rust to GDExtension is an option to consider
• I think GDExtension should be a compilation target only
• Exposing the AST should not be accounted for for this

Compiling rust to GDExtension is an option to consider

I didn't think about it yesterday when you brought this up on Discord, but actually we can compile rust code to a GDExtension without rewriting into another language. It's worth at least considering and evaluating before rewriting the entire thing into another language because it's probably only a small fraction of the work (and no risk of regressions).

The formatter is already a library that can be wrapped around. The CLI itself only imports the formatter and linter for compilation. Godot rust gdextension support: https://godot-rust.github.io/

Given that, a complete rewrite in C++ should be motivated before proceeding (I wouldn't want anyone to take on that much workload and risk facing many regressions for little practical gain). I've considered rewriting in C++ before rewriting the formatter in Rust, in case the Godot team would eventually want this to be an official part of Godot. For now it's not the case/it seems they'd rather keep this external anyway.

Another reason is using the same toolchain and coding practices as Godot for companies who'd want to customize the formatter alongside the engine. But that, like exposing the AST, is the kind of thing that almost nobody uses in practice; there have been no requests or needs expressed in that direction.

I think GDExtension should be a compilation target only

Users rely on the CLI; removing the CLI and making the program GDExtension-only (for example, by using Godot APIs and types for formatting itself) would make running the formatter through the CLI 3 to 4 orders of magnitude slower because you'd have to spawn a copy of Godot each time. Even headless it's a couple hundred ms at least on a fast computer.

It'd also be a breaking change to user workflows (requiring adding a copy of Godot in CI, changing their format commands in external code editors, etc.).

As a bonus it's way less work to keep the existing cli, formatter/linter libs, and compilation target. And it's possibly already compatible with gdextension.

Exposing the AST should not be accounted for for this

For the reason briefly mentioned above + it's unrelated to the formatter. It shouldn't come into account when considering turning the formatter into a gdextension. It's a separate potential project and body of work that's not necessarily reduced by compiling the formatter as a GDExtension or rewriting it.

---

I'm bringing up these points based on the state of the conversation. We're left with 2 benefits for users from a GDExtension, for users of the Godot addon only:

1. No need to run one command to download the formatter anymore.
2. The formatter doesn't run as a separate process anymore. That's generally transparent to users. For most people it'd shave off ~15ms running time on Windows and ~1-3ms on unix systems. (note: If the issue is the cost of spawning the process on Windows, the formatter could be tweaked to work as a daemon; that's not a lot of work and would address this part)

That comes at the cost of extra initial work (at least writing the addon in GDExtension, testing on different platforms...) probably some extra maintenance, and more compilation time (can't iterate on the addon and test it instantly anymore).

I'm open to it and I appreciate it if you do want to take on this workload for improving user experience in that way. I'm underlying this just so you're sure that you want to put in the time for these. A C++ programmer came by recently eager to port the formatter to C++ collaboratively and quickly abandoned the project. I would imagine upon realizing the actual scope of the work.

We can discuss the implementation and estimate the time different approaches would take if you'd like. In any case, I'd like to discuss and agree on the implementation before you invest a lot of time into it.

With that said, I want to thank you for bringing this up and considering contributing.

[khellste]

I hadn't considered godot-rust but it sounds like it might be the best way to avoid a massive rewrite full of regressions.

I have no Rust experience, but from a little Googling it seems it might look something like this:

• Use gdext to write a thin GDExtension wrapper around the formatter. Here is a guide for my reference.
• I can start with a basic singleton interface. Something like GDScriptFormatter.format_gdscript(source: String, options: Dictionary) -> String would be a good start.
• Pause here to see how well that worked, and worry about linter/addon support later.

Thoughts?

[NathanLovato]

Starting there to see how well it works is a good plan.

If you write it procedural rust code and avoid using too many abstractions the language provides it's pretty close to other systems programming languages minus the memory allocation code (memory allocations are implied when you instantiate a struct or Vec etc., done on the stack or heap for you). If you have any questions or need a hand along the way please reach out anytime!

The main things that can be a bit hard at first is the concept of owning borrowing and how rust constrains how you can access and mutate things as part of its compile time memory access checks. But I'm not sure you'll run much into it with a GDExtension, interfacing with Godot (most of the code will touch things on the Godot side which manages its own memory).

[khellste]

Well, I have a proof of concept that was surprisingly easy to make (<150 LOC): https://github.com/khellste/GDScript-formatter/. Kudos to the godot-rust team for that.

A few open questions:

• It adds a godot-rust dependency. Should this be forked like you did for tree-sitter-gdscript?
• The exposed method accepts formatter options as a Dictionary, and the code that translates from Dictionary to FormatterConfiguration is very ugly. Suggestions welcome. (We could expose the formatter options as part of the API, but I would be pretty hesitant about "infecting" the rest of the project with #[godot_whatever] annotations. Maybe there's a way to do it conditionally? Or maybe someone with more godot-rust experience has some tips?)
• What's the best way to release the .gdextension file, which requires a fixed resource path for the built gdscript_formatter_gdextension.so? Should there be a separate version of the addon that includes the GDExtension?
• How does your CI system work? Pardon my ignorance, I'm pretty bad at Github.

[NathanLovato]

It's looking good! Thanks for your work.

It adds a godot-rust dependency. Should this be forked like you did for tree-sitter-gdscript?

No, the only reason I fork the Tree Sitter parser is that while being a maintainer, for more sensitive changes, I submit them as pull requests and Preston, the original author of the project, has very limited availability to review and merge them. With the fork the formatter can immediately use changes to the parser.

the code that translates from Dictionary to FormatterConfiguration is very ugly

Not at all! It's very straightforward code and it's completely normal to marshall data between Godot and rust. One potential but not essential improvement would be storing the different settings into a structured and typed object so you'd already have the fields validated and we can see what are the valid fields coming from the Godot side.

What's the best way to release the .gdextension file, which requires a fixed resource path for the built gdscript_formatter_gdextension.so? Should there be a separate version of the addon that includes the GDExtension?

In short yes but I can't say much about the details, I've used GDNative back in the day but GDExtension only tested briefly. What's for sure is it needs to exist separately from the existing addon until it covers its feature set, as a more native addon option. For distribution we'll add a job to the CI to cross compile and upload copies of the gdextension in whatever form is necessary. Then once it reaches full featture parity/it covers all use cases of the existing GDScript addon and we know for sure it works reliably for end users, we can consider deprecating and eventually removing the add-on in favor of it.

How does your CI system work? Pardon my ignorance, I'm pretty bad at Github.

I can take care of that part. You don't have to necessarily worry about it/learning the specifics. On GitHub it works by defining a build pipeline with a declarative syntax, you define jobs and dependencies, these jobs being little programs. Github runs the jobs (programs) based on the dependencies you've declared. Some variables in there like $matrix allow you to run the same pipeline in parallel for multiple build targets (one for windows x64, one for ARM-based macos, etc.).