Whitespace #60
Replies: 7 comments
-
|
We may be able to add a GH Actions workflow that will automatically forcibly convert all tabs to spaces in code files (if I'm understanding correctly that "no tabs" is the unambiguously technically correct solution, rather than personal preference). Or, less forcefully, throw up an error if a PR introduces tabs, but if we can safely automate it that seems more approachable to newcomers. |
Beta Was this translation helpful? Give feedback.
-
|
Since the error is caused by Godot automatically adding tabs into space-delimited files I'd be inclined to go the other way and enforce tabs, but I've also seen Main.gd (for example) using two spaces for indentation instead of four, so it'd be ambiguous what should count as a full indent or not. There's definitely not a technically correct solution here, it's just whatever makes it easiest to onboard people. |
Beta Was this translation helpful? Give feedback.
-
|
Okay, cool, I thought you were saying there was a specific reason that GDScript in particular needs spaces instead of tabs, as opposed to it sounding like the usual subjective arguments apply here. Still sounds to me like, if possible, an auto-formatter GH Actions step would be the correct call, and one that needs to be smart enough to handle intelligently detecting indent levels. |
Beta Was this translation helpful? Give feedback.
-
|
I'm not Godot-savvy enough to judge how sensible these conventions are, but this tool looks like it would Just Work if we wired it to run on every PR: https://github.com/Scony/godot-gdscript-toolkit Presumably the flow would be:
This way, we can test to make sure that the bot didn't make failing changes before merging into main. This does place the onus of fixing that on whoever opened the PR, but I think it's reasonable to say "you're responsible for writing code that doesn't fuck with the formatter" (at least assuming the formatter is decently-written). In other environments I'd prefer to have this run as some sort of pre-commit hook instead of happening on GitHub, but I think this is going to be the simplest path for a project like this, since we prioritize not wanting people to have to work through a big checklist of action items before opening a PR. That godot-gdscript-toolkit uses https://pre-commit.com to auto-configure git pre-commit hooks, but we absolutely don't want to require contributors to have a local functioning Python dev environment |
Beta Was this translation helpful? Give feedback.
-
|
I think this is reasonable, yeah. |
Beta Was this translation helpful? Give feedback.
-
|
We should also set up an |
Beta Was this translation helpful? Give feedback.
-
|
Given the I think we should start with just running the fixer when code is pushed to main. If that ends up causing flakiness, we can investigate the more complicated workflow then. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
GDScript takes a lot of its design from Python, including the syntactic whitespace, which is already becoming an issue (ref #54). We are probably going to have to do something about the whitespace.
Unfortunately there's no obvious 'good' solution because the Godot editor is not sophisticated enough to handle the issue on its own, and comes with defaults that make the problem worse in this case (automatically converting four spaces to a tab, which breaks space-indented files).
For now, though, we should probably at least make sure everyone has this option turned off, because it will continue to generate problems:

Beta Was this translation helpful? Give feedback.
All reactions