godot-gdscript-toolkit icon indicating copy to clipboard operation
godot-gdscript-toolkit copied to clipboard

gdformat creates a line longer than 100 symbols and gdlint complains about it

Open AndreyNautilus opened this issue 3 years ago • 3 comments
trafficstars

In my project I run both gdlint and gdformat and I found something what seems like a bug.

I have a file with the following content:

static func on_quest_finished(_state: GameState, _quest_type: String, _iker_id: int, _etadata: Dictionary):
	pass

The line with function declaration is 107 characters long, so gdlint complains about it, but gdformat is perfectly fine with this code:

$ gdlint tmp/test.gd
tmp/test.gd:1: Error: Max allowed line length (100) exceeded (max-line-length)
Failure: 1 problem found
$ gdformat tmp/test.gd  --diff
1 file would be left unchanged

If I manually change the code so it fits in 100 symbols:

static func on_quest_finished(
	_state: GameState, _quest_type: String, _iker_id: int, _etadata: Dictionary
):
	pass

gdlint is happy, but gdformat wants to revert it to oneline (and violate the 100 length rule):

$ gdlint tmp/test.gd
Success: no problems found
$ gdformat tmp/test.gd  --diff
would reformat tmp/test.gd
--- tmp/test.gd
+++ tmp/test.gd
@@ -1,4 +1,2 @@
-static func on_quest_finished(
-	_state: GameState, _quest_type: String, _iker_id: int, _etadata: Dictionary
-):
+static func on_quest_finished(_state: GameState, _quest_type: String, _iker_id: int, _etadata: Dictionary):
 	pass
1 file would be reformatted, 0 files would be left unchanged.

If I make the line in the original file 1 symbol longer (note _metadata vs _etadata):

static func on_quest_finished(_state: GameState, _quest_type: String, _iker_id: int, _metadata: Dictionary):
	pass

both gdlint and gdformat start to complain about it (as expeected):

$ gdlint tmp/test.gd
tmp/test.gd:1: Error: Max allowed line length (100) exceeded (max-line-length)
Failure: 1 problem found
$ gdformat tmp/test.gd  --diff
would reformat tmp/test.gd
--- tmp/test.gd
+++ tmp/test.gd
@@ -1,2 +1,4 @@
-static func on_quest_finished(_state: GameState, _quest_type: String, _iker_id: int, _metadata: Dictionary):
+static func on_quest_finished(
+	_state: GameState, _quest_type: String, _iker_id: int, _metadata: Dictionary
+):
 	pass
1 file would be reformatted, 0 files would be left unchanged.

The workaround is to silence gdlint with # gdlint:ignore = max-line-length, so it's not super-critical, but still it's a bug.

AndreyNautilus avatar Jul 21 '22 13:07 AndreyNautilus

looks like some corner case that needs to be addressed - thanks for reporting it

Scony avatar Jul 21 '22 19:07 Scony

I can try to have a look at my free time if you don't mind.

AndreyNautilus avatar Jul 29 '22 17:07 AndreyNautilus

@AndreyNautilus no need, I already know what's the problem - have to find a proper way to resolve it

Scony avatar Jul 29 '22 20:07 Scony

The formatter and linter also don't play nice with line continuations. Contrived example:

match x:
  "one", "two", "three", "four", "five", .... (and so on)

If I fix the linter long line complaint using this:

match x:
  "one", "two", "three", "four", "five", \
  ... etc. \

Then the linter is happy, but the formatter wants to put the lines back into one. Thanks for all your efforts on these tools, they are generally very helpful!

ngburke avatar Dec 04 '22 02:12 ngburke

@ngburke the thing is - patterns are not fully implemented i.e. formatter always formats them into one line. This yields lines invalid from gdlint PoV.

Scony avatar Dec 04 '22 13:12 Scony

@AndreyNautilus multiline patterns are now supported, although the list pattern i.e.

match x:
  "one", "two", "three", "four", "five", .... (and so on)

cannot be broken into multiple lines in an elegant way due to Godot's limitations.

Please note that the above can be rewritten as:

if x in ["one", "two", three"]:

and the formatter will be able to break such array expression into multiple lines.

Regarding original issue with static keyword, it should be working on master by now.

Please retest and reopen this issue if required.

Scony avatar Dec 06 '23 21:12 Scony