Fix bad formatting on several operators

[DaelonSuzuka]

Fixes #601, #602, and #609

Before this is merged, it's probably worth going through all of GDScripts operators and making sure they're represented in the snapshot tests.

[Calinou]

Note that in master, this is also currently being incorrectly formatted:

func update_preset() -> void:
    # Simulate options being manually selected to run their respective update code.
    %TAAOptionButton.item_selected.emit(%TAAOptionButton.selected)
    %MSAAOptionButton.item_selected.emit(%MSAAOptionButton.selected)
    %FXAAOptionButton.item_selected.emit(%FXAAOptionButton.selected)
    %ShadowSizeOptionButton.item_selected.emit(%ShadowSizeOptionButton.selected)
    %ShadowFilterOptionButton.item_selected.emit(%ShadowFilterOptionButton.selected)
    %MeshLODOptionButton.item_selected.emit(%MeshLODOptionButton.selected)
    %SDFGIOptionButton.item_selected.emit(%SDFGIOptionButton.selected)
    %GlowOptionButton.item_selected.emit(%GlowOptionButton.selected)
    %SSAOOptionButton.item_selected.emit(%SSAOOptionButton.selected)
    %SSReflectionsOptionButton.item_selected.emit(%SSReflectionsOptionButton.selected)
    %SSILOptionButton.item_selected.emit(%SSILOptionButton.selected)
    %VolumetricFogOptionButton.item_selected.emit(%VolumetricFogOptionButton.selected)

The formatter will turn this into:

func update_preset() -> void:
    # Simulate options being manually selected to run their respective update code.
    %TAAOptionButton.item_selected.emit( %TAAOptionButton.selected)
    %MSAAOptionButton.item_selected.emit( %MSAAOptionButton.selected)
    %FXAAOptionButton.item_selected.emit( %FXAAOptionButton.selected)
    %ShadowSizeOptionButton.item_selected.emit( %ShadowSizeOptionButton.selected)
    %ShadowFilterOptionButton.item_selected.emit( %ShadowFilterOptionButton.selected)
    %MeshLODOptionButton.item_selected.emit( %MeshLODOptionButton.selected)
    %SDFGIOptionButton.item_selected.emit( %SDFGIOptionButton.selected)
    %GlowOptionButton.item_selected.emit( %GlowOptionButton.selected)
    %SSAOOptionButton.item_selected.emit( %SSAOOptionButton.selected)
    %SSReflectionsOptionButton.item_selected.emit( %SSReflectionsOptionButton.selected)
    %SSILOptionButton.item_selected.emit( %SSILOptionButton.selected)
    %VolumetricFogOptionButton.item_selected.emit( %VolumetricFogOptionButton.selected)

I'm not sure if fixing this is within the scope of this PR, but I figured I'd mention it nonetheless.

[DaelonSuzuka]

That's awesome, thanks @Calinou!

[Mr-Byte]

I ran into an issue that seems related where the following code:

func test(x):
    match x:
         var y when y > 20:
            pass

Gets formatted into:

func test(x):
    match x:
         var ywheny > 20:
            pass

Which unfortunately causes the formatted code to become invalid.

[DaelonSuzuka]

I don't think I've seen a "when" keyword in GDScript. Thanks for reporting!

[DaelonSuzuka]

@Calinou it looks like when isn't in the list of keywords here: https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_basics.html#keywords

Is this table supposed to be exhaustive?

[DaelonSuzuka]

The when keyword is now syntax highlighted and formatted correctly.

[DaelonSuzuka]

%Unique node shorthand inside parens now formats correctly. (Actually, everything following an open paren should format correctly now.)

[DaelonSuzuka]

Working on fix for #612:

This breaks half the tests, it doesn't properly handle comments, and it doesn't insert blank lines if they're missing.

[jogly]

I also confirmed this fixes the currently broken formatting of the bitwise operators >> and <<! ty <3

[rktprof]

Heads up, this breaks some variable names, namely variables starting with "and" or "or" gets formatted

for example:

func _ready() -> void:
    var origin: Vector2 = Vector2.ZERO
    origin.x = 1

becomes:

func _ready() -> void:
    var origin: Vector2 = Vector2.ZERO
    or igin.x = 1

[DaelonSuzuka]

@rktprof Thanks for the report.

[DaelonSuzuka]

@rktprof

Fixed the issue with and/or/not in identifiers:

[DaelonSuzuka]

Fixed consecutive trailing line removal:

[DaelonSuzuka]

Fixed bad bitwise operator formatting:

[rktprof]

Noticed another issue in this PR when using .rpc()

Whenever you save it swaps (as in keep saving and it keeps swapping) between and:

[rktprof]

Another thing that is a bit odd is that I have classes call UIScreen and UIPopup and they get incorrect syntax coloring in some cases.

Some comparisons between custom classes

[DaelonSuzuka]

Noticed another issue in this PR when using .rpc()

Whenever you save it swaps (as in keep saving and it keeps swapping) between

rpc is a language keyword so I'm surprised that's a valid identifier...

[rktprof]

Noticed another issue in this PR when using .rpc() Whenever you save it swaps (as in keep saving and it keeps swapping) between

rpc is a language keyword so I'm surprised that's a valid identifier...

Yeah, no idea why it gets special treatment. I haven't seen it on call(), call_deferred() or any others I've used in this project, only rpc().

And I should clarify that it's not new for this PR, it just still happens since 2.0.0.

[DaelonSuzuka]

Yeah, no idea why it gets special treatment. I haven't seen it on call(), call_deferred() or any others I've used in this project, only rpc().

And I should clarify that it's not new for this PR, it just still happens since 2.0.0.

No, you misunderstand. rpc is a reserved keyword in the language. call and call_deferred are not keywords, they're builtin functions. I would have expected rpc() to be an invalid name for a function, just like if() and while() would be.

[rktprof]

No, you misunderstand. rpc is a reserved keyword in the language. call and call_deferred are not keywords, they're builtin functions. I would have expected rpc() to be an invalid name for a function, just like if() and while() would be.

Ah, rpc() is the built-in function for network calls, not something I created

Edit: There's also rpc_id() which does not have the same formatting issue as rpc()

[DaelonSuzuka]

No, you misunderstand. rpc is a reserved keyword in the language. call and call_deferred are not keywords, they're builtin functions. I would have expected rpc() to be an invalid name for a function, just like if() and while() would be.

Ah, rpc() is the built-in function for network calls, not something I created

Edit: There's also rpc_id() which does not have the same formatting issue as rpc()

@rktprof You're absolutely right and I'm an idiot. Somehow I mixed up the 3 groups of rpc related keywords (early 3.x, late 3.x, and 4.x) and combined that with the fact that rpc() and rpc_id() are highlighted as keywords instead of functions.

I think this is actually an easy fix.

edit: even worse, rpc() is highlighted as a keyword but rpc_id() isn't!

[DaelonSuzuka]

@rktprof thing.rpc() should behave properly now. I just dropped the special-case highlighting.

Your class names should also behave better:

[qt911025]

godot version 4.2 vscode plugin version 2.0.0

A space is added between preload and parentheses.

And resource path's completion only happens inside quotes, but redundant quotes is added after completion.

[DaelonSuzuka]

@qt911025 Yes, that's already been fixed in this PR.

[limbonaut]

Not sure if it's a known issue. return -1 is reformatted as return - 1

[DaelonSuzuka]

@limbonaut Thanks for the report. That's been added to the test suite and it should be behaving correctly now.

[rktprof]

I noticed another thing that would be nice to have; differentiating code in strings, like in this example:

The equivalent in C# is

Important to note is that not having $ at the start of a string in C# or .format() at the end of a string in GDScript should not give any special treatment.

I would want either the brackets to be coloured as operators or the string to be treated as a variable. It's never going to be as clear in GDScript as in C# but that is a problem with the language itself

Edit: I should add that it's not entirely equivalent, as C# also has it's own similar format, but I would argue that it should have some syntax highlighting as well

[DaelonSuzuka]

I noticed another thing that would be nice to have; differentiating code in strings, like in this example:

Wait, that's valid GDScript? I've never seen .format() in Godot.

Syntax highlighting improvement is technically off-topic for this PR, but I'll look into this.

[rktprof]

Wait, that's valid GDScript? I've never seen .format() in Godot.

Syntax highlighting improvement is technically off-topic for this PR, but I'll look into this.

Yeah sorry, I realised after posting that it should probably be a separate issue but instead of deleting and recreating I just left it.

As for the actual script, yeah, I find it way too cumbersome to actually use in most cases but I recently wanted to insert a bunch of colors in a BBCode string and found it useful there. Link to the docs for posterity.

[DaelonSuzuka]

Brace placeholders should now be formatted correctly:

[limbonaut]

Formatter stops working the moment I add a subclass:

class X:
    var y

BTW, should I open separate issues, or is it fine dropping these issues here?

[DaelonSuzuka]

BTW, should I open separate issues, or is it fine dropping these issues here?

Putting them here is perfect, thanks!

subclasses

Wierd, there's subclasses in the example files I thought it formatted them just fine. I'll make sure there are subclasses in the test suite.

[limbonaut]

Wierd, there's subclasses in the example files I thought it formatted them just fine. I'll make sure there are subclasses in the test suite.

It is kinda weird. If I put two new-line characters in front of the class definition, formatter runs as expected. But if I put a single new-line character, it stops doing anything :shrug:

# file_a.gd: Where formatter runs as expected.
extends Object

class X:
    var y:int

# file_b.gd: Where formatter doesn't do anything.
extends Object

class X:
    var y:int

[DaelonSuzuka]

Oh awesome, I know exactly what's causing that. Thanks for the examples, that narrowed it down perfectly!

[rktprof]

Found another little annoyance

@export var my_value:int = 0 ## Description of this int

Gets formatted into:

@export var my_value:int = 0 # # Broken Description of this int

This still works:

## Description of this int
@export var my_value:int = 0

Edit: Additionally, it will always add a space after the # for inline comments, but the code style guidelines says that code comments should follow immediately after the #

@export var my_value:int = 0 #1

Becomes

@export var my_value:int = 0 # 1

[Nitwel]

Any chance we could get a new release soon as although this PR might not have caught every edge case, having a release with most bugs fixed would already improve the DX substantially. Sorry for being off topic but some of these bugs have driven me crazy recently.

[Calinou]

The PR needs to be merged first before we can release 2.1.0 :slightly_smiling_face:

I don't know if this PR is currently in a mergeable state. @DaelonSuzuka Do you think we could try to merge it now?

Once it's merged, we should be able to release 2.1.0 immediately after.

[limbonaut]

Don't think it is ready. There is an option for one or two lines between functions, and it doesn't yet work. Not for me at least - it's always one line.

[limbonaut]

Wanted to report another reformatting accident: func _add_action(p_action: StringName, p_key: Key, p_alt: Key = KEY_NONE) -> void: => func _add_action(p_action: StringName, p_key: Key, p_alt: Key=KEY_NONE) -> void:

Doesn't look right to me.

[DaelonSuzuka]

@Calinou

@limbonaut is right, this PR isn't ready. There's a number of outstanding problems but I should finally have some free time to work on it this week.

@limbonaut

Wanted to report another reformatting accident: func _add_action(p_action: StringName, p_key: Key, p_alt: Key = KEY_NONE) -> void: => func _add_action(p_action: StringName, p_key: Key, p_alt: Key=KEY_NONE) -> void:

Doesn't look right to me.

This is actually not a bug, I prefer removing the whitespace inside parameter lists for aesthetic/space reasons, and because that's the idiomatic style for several other languages I use.

However, enough people have thought this was a bug that I'm considering changing it.

[limbonaut]

@DaelonSuzuka It only looks strange with static typing. When no types provided, that looks fine, like in some other languages that support default argument values. With types, it kinda looks random, though.

I checked with black - python's formatter - to see how it handles such cases. It removes whitespace when no types involved, and leaves single space when types are provided.

def func(arg=5):
    pass

def func(arg: int = 5):
    pass

Hope it helps.

[DaelonSuzuka]

I have previously stated that I'm against configuration options, but I think that's the least wrong thing to do here. There's now two densities for function (and lambda) parameter lists, dense and not dense. The default is dense==false, which matches the Godot style guide. The dense mode should match what black does (thanks to @limbonaut for correcting my faulty memory about my main programming language T_T).

[phelioz]

Formater breaks bitwise OR assignment operator. Turns:

test |= 1

Into:

test | = 1

[limbonaut]

With the latest build and two empty lines setting, I've got the following issue related to comments:

extends Node
## Class doc

func _ready() -> void:
    pass

func no_comment() -> void:
    pass

## Doc comment
func with_doc_comment() -> void:
    pass

# Comment
func with_comment() -> void:
    pass

## Multiline doc comment
## Second line
func with_multiline_doc_comment() -> void:
    pass

static func a_static_func() -> void:
  pass

Reformatted to:

extends Node
## Class doc

func _ready() -> void:
    pass

func no_comment() -> void:
    pass

## Doc comment
func with_doc_comment() -> void:
    pass

# Comment
func with_comment() -> void:
    pass

## Multiline doc comment
## Second line
func with_multiline_doc_comment() -> void:
    pass

static func a_static_func() -> void:
  pass

So it seems that two lines setting work, unless a function declaration has comments preceding.

UPDATE: and also static functions don't respect two-lines setting.

[limbonaut]

Negative indexing issue - space introduced

_array[-1] = 0 → _array[- 1] = 0

Typed array arguments don't follow density rules

func _init(m: Array[int] = []) -> void:
↓↓↓
func _init(m: Array[int]=[]) -> void:

Inline documentation comments get broken

enum MyEnum { ## My enum.
    VALUE_A = 0, ## Value A.
    VALUE_B = 1, ## Value B.
}

↓↓↓

enum MyEnum { # # My enum.
    VALUE_A = 0, # # Value A.
    VALUE_B = 1, # # Value B.
}

Negative numbers in function arguments

func negnum(number = -1) -> void:
↓↓↓
func negnum(number =- 1) -> void:

Note: Same issue if types are specified.

[limbonaut]

Here's all these issues in a single GDScript file: test_formatter.zip

[DaelonSuzuka]

Alright, @limbonaut's test cases are (almost) all implemented. It's time to merge this and start arranging a new release.

I'm sure there will be a new "general formatter work" PR soon, so additional feedback can either open new issues or wait for the new thread.