Scony / godot-gdscript-toolkit

Independent set of GDScript tools - parser, linter, formatter, and more
MIT License
999 stars 68 forks source link

Formatter doesn't format lines to linter's liking #306

Open ilyvion opened 5 months ago

ilyvion commented 5 months ago
❯ gdformat test.gd
0 files reformatted, 1 file left unchanged.

❯ gdlint test.gd
test.gd:2: Error: Max allowed line length (100) exceeded (max-line-length)
Failure: 1 problem found

Here's test.gd from above:

func test():
    neighbor_debug_text.label_stack[neighbor_debug_text.label_stack.size() - 1] = "CHOSEN NEXT NEIGHBOR"

Even if you ignore tab width, that line is 102 characters long, so it's odd that the formatter leaves it alone.

Tested with both latest release version (4.2.2) and latest master branch commit (which reports 4.2.3):

❯ gdformat --version
gdformat 4.2.2
❯ gdlint --version
gdlint 4.2.2
❯ gdlint --version
gdlint 4.2.3

❯ gdformat --version
gdformat 4.2.3
Scony commented 5 months ago

Well, the formatter in it's current state is unable to break the above line in any way and therefore it's formatting to single line. There are more such cases i.e. consider this code:

var x = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

the formatter cannot really do anything about it and therefore the formatted line will be long anyway and linter will complain.

In other words - formatter doesn't guarantee the line will be reduced under the threshold specified.

Anyway, back to your case - if formatter would be more aggressive here it could break the line in a few ways. I'll try to improve it.

ilyvion commented 5 months ago

What I ended up doing in the meantime was manually change the line after auto-formatting to

neighbor_debug_text.label_stack[
    neighbor_debug_text.label_stack.size() - 1
] = "CHOSEN NEXT NEIGHBOR"

which is at least accepted by Godot as valid.

imberny commented 1 month ago

Well, the formatter in it's current state is unable to break the above line in any way and therefore it's formatting to single line.

That doesn't seem to be the only explanation. Consider this line:

@onready var long_string := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

which is 101 characters long. If I run the formatter with --line-length=100 it leaves it as is (and the linter complains), but if I run it with --line-length=99 if produces the following:

@onready
var long_string := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

showing that it is capable of handling this case but chooses not too. Off by one error?

Scony commented 1 month ago

Well, the formatter in it's current state is unable to break the above line in any way and therefore it's formatting to single line.

That doesn't seem to be the only explanation. Consider this line:

@onready var long_string := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

which is 101 characters long. If I run the formatter with --line-length=100 it leaves it as is (and the linter complains), but if I run it with --line-length=99 if produces the following:

@onready
var long_string := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

showing that it is capable of handling this case but chooses not too. Off by one error?

This one looks like a bug indeed.