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

Formatter only keeps 1 blank line between functions, but the GDScript style guide mandates 2 #612

Open
Calinou opened this issue Feb 29, 2024 · 10 comments
Labels

Comments

@Calinou
Copy link
Member

Calinou commented Feb 29, 2024

Godot version

4.2.1.stable

VS Code version

1.86.2

Godot Tools VS Code extension version

2.0.0

System information

Fedora 39

Issue description

The GDScript formatter only keeps 1 blank line between functions, but the GDScript style guide mandates 2.

func _on_sdfgi_option_button_item_selected(index: int) -> void:
	# This is a setting that is attached to the environment.
	# If your game requires you to change the environment,
	# then be sure to run this function again to make the setting effective.
	if index == 0: # Disabled (default)
		world_environment.environment.sdfgi_enabled = false
	if index == 1: # Low
		world_environment.environment.sdfgi_enabled = true
		RenderingServer.gi_set_use_half_resolution(true)
	if index == 2: # High
		world_environment.environment.sdfgi_enabled = true
		RenderingServer.gi_set_use_half_resolution(false)


func _on_glow_option_button_item_selected(index: int) -> void:
	# This is a setting that is attached to the environment.
	# If your game requires you to change the environment,
	# then be sure to run this function again to make the setting effective.
	if index == 0: # Disabled (default)
		world_environment.environment.glow_enabled = false
	if index == 1: # Low
		world_environment.environment.glow_enabled = true
	if index == 2: # High
		world_environment.environment.glow_enabled = true

Is turned into:

func _on_sdfgi_option_button_item_selected(index: int) -> void:
	# This is a setting that is attached to the environment.
	# If your game requires you to change the environment,
	# then be sure to run this function again to make the setting effective.
	if index == 0: # Disabled (default)
		world_environment.environment.sdfgi_enabled = false
	if index == 1: # Low
		world_environment.environment.sdfgi_enabled = true
		RenderingServer.gi_set_use_half_resolution(true)
	if index == 2: # High
		world_environment.environment.sdfgi_enabled = true
		RenderingServer.gi_set_use_half_resolution(false)

func _on_glow_option_button_item_selected(index: int) -> void:
	# This is a setting that is attached to the environment.
	# If your game requires you to change the environment,
	# then be sure to run this function again to make the setting effective.
	if index == 0: # Disabled (default)
		world_environment.environment.glow_enabled = false
	if index == 1: # Low
		world_environment.environment.glow_enabled = true
	if index == 2: # High
		world_environment.environment.glow_enabled = true

Note that this only applies between functions/classes, not between top-level member variables and functions/classes.

Steps to reproduce

Format the above code using Ctrl + Shift + I.

@Calinou Calinou added the bug label Feb 29, 2024
@DaelonSuzuka
Copy link
Collaborator

Technically this isn't a bug: I think two newlines between functions is bad and the official style guide is wrong.

But... I recognize that I'm the weirdo here, so I'll change the default behavior to do 2 newlines.

@limbonaut
Copy link

Wouldn't it be nice to have a configuration option?

@DaelonSuzuka
Copy link
Collaborator

I've been trying to avoid adding options for the formatter because that dramatically increases the complexity of both the formatter and the test suite.

This probably will be an option, because I really don't want two newlines in my projects.

@DaelonSuzuka
Copy link
Collaborator

Code_BQ519Gn3mh

Progress. It doesn't handle comments/doc comments yet, and it doesn't insert blanks lines if they're missing. And it breaks half the tests.

@chrisl8
Copy link

chrisl8 commented Mar 8, 2024

Technically this isn't a bug: I think two newlines between functions is bad and the official style guide is wrong.

But... I recognize that I'm the weirdo here, so I'll change the default behavior to do 2 newlines.

FWIW, I think Godot is attempting to follow the Python style guide.

https://peps.python.org/pep-0008/#blank-lines

Surround top-level function and class definitions with two blank lines.

@Calinou
Copy link
Member Author

Calinou commented Mar 11, 2024

FWIW, I think Godot is attempting to follow the Python style guide.

Indeed, that was the main rationale for using 2 blank lines between functions/classes. There's also the reason that it mimics the effective spacing between functions in languages with brace syntax:

void something() {
	// pass
}

void other_function() {
	// pass
}
def something():
	pass


def other_function():
	pass

In both examples above, there are 2 "blank" lines between functions (even if in the brace-based case, there's a lone brace on the first line).

@rktprof
Copy link

rktprof commented Mar 14, 2024

Would it be possible to make it add lines but never remove lines? Sometimes I like to separate blocks with more than 1 or 2 lines

@Calinou
Copy link
Member Author

Calinou commented Mar 14, 2024

Would it be possible to make it add lines but never remove lines? Sometimes I like to separate blocks with more than 1 or 2 lines

It's possible to design a formatter this way, but I don't think it's a good idea as it goes against the style guide (on top of making the formatter code more complex and bug-prone).

@rktprof
Copy link

rktprof commented Mar 15, 2024

It's possible to design a formatter this way, but I don't think it's a good idea as it goes against the style guide (on top of making the formatter code more complex and bug-prone).

Understandable, in this specific case it just feels more like a style enforcement rather than a guide =)

@limbonaut
Copy link

In defense of two blank lines... I tend to use blank lines to break up longer functions into logical subsections. With a single blank line between functions, it becomes harder to see at a glance where one function ends and another starts.

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

No branches or pull requests

5 participants