Add a GitHub "Action" to enforce code style

[SirMoM]

As proposed in #12. Let's discuss the results.

[akien-mga]

That's not an improvement IMO.

[SirMoM]

I agree. This happens because the line length is capped at 100. I could change the command to:
gdformat --line-length=500 logger.gd
This should solve this kind of formatting!

[akien-mga]

That's a weird style change. Maybe replace ! with not and then it makes sense.

[SirMoM]

Yes this is still the case and an issue was created: Scony/godot-gdscript-toolkit#106
How do we want to approach this?

[akien-mga]

I'd say work it around with not instead of !, which is also more pythonic.

[akien-mga]

Does it keep them split if you add a trailing comma?

[SirMoM]

Yes it does!

[akien-mga]

I'd say it would be good to do then, it's more readable IMO.

[akien-mga]

Try to add a trailing comma to keep them on separate lines.

[akien-mga]

Well... Autoformatters are dumb :)
I guess we can live with it.

[SirMoM]

Right now the linter will always fail due to its configuration:

class-definitions-order:
- tools
- classnames
- extends
- signals
- enums
- consts
- exports
- pubvars
- prvvars
- onreadypubvars
- onreadyprvvars
- others
class-load-variable-name: (([A-Z][a-z0-9]*)+|_?[a-z][a-z0-9]*(_[a-z0-9]+)*)
class-name: ([A-Z][a-z0-9]*)+
class-variable-name: _?[a-z][a-z0-9]*(_[a-z0-9]+)*
comparison-with-itself: null
constant-name: '[A-Z][A-Z0-9]*(_[A-Z0-9]+)*'
disable: []
duplicated-load: null
enum-element-name: '[A-Z][A-Z0-9]*(_[A-Z0-9]+)*'
enum-name: ([A-Z][a-z0-9]*)+
expression-not-assigned: null
function-argument-name: _?[a-z][a-z0-9]*(_[a-z0-9]+)*
function-arguments-number: 10
function-load-variable-name: ([A-Z][a-z0-9]*)+
function-name: (_on_([A-Z][a-z0-9]*)+(_[a-z0-9]+)*|_?[a-z][a-z0-9]*(_[a-z0-9]+)*)
function-variable-name: '[a-z][a-z0-9]*(_[a-z0-9]+)*'
load-constant-name: (([A-Z][a-z0-9]*)+|[A-Z][A-Z0-9]*(_[A-Z0-9]+)*)
loop-variable-name: _?[a-z][a-z0-9]*(_[a-z0-9]+)*
max-file-lines: 1000
max-line-length: 100
max-public-methods: 20
mixed-tabs-and-spaces: null
private-method-call: null
signal-name: '[a-z][a-z0-9]*(_[a-z0-9]+)*'
sub-class-name: _?([A-Z][a-z0-9]*)+
trailing-whitespace: null
unnecessary-pass: null
unused-argument: null

To make the linter somewhat useful I propose to disable: disable: [class-definitions-order, max-file-lines, max-line-length, max-public-methods]

Is this something I should do?
Or how should we use the linter?

[akien-mga]

Would it keep things multiline if written this way?

Suggested change

	var module = Module.new(module_cfg["name"], module_cfg["output_level"], module_cfg["output_strategies"], get_logfile(module_cfg["logfile_path"]))
	var module = Module.new(
	module_cfg["name"],
	module_cfg["output_level"],
	module_cfg["output_strategies"],
	get_logfile(module_cfg["logfile_path"])
	)

or similar?

[SirMoM]

No, this will be formatted to one line.

[akien-mga]

Sorry for the delay. Yeah disabling what you need for the linter to pass seems good. We can always assess the options that we had to disable in a second step once we have a working formatter and linter merged.

As for the max line length, you can experiment with e.g. 120 or 140 (we use 120 for Python formatting in Godot code for example) to avoid splitting everything needlessly, but still split the occasional very long lines. But if the output is not good, I'm also fine with setting 500 to be virtually unlimited.

[SirMoM]

As for the max line length, you can experiment with e.g. 120 or 140 (we use 120 for Python formatting in Godot code for example) to avoid splitting everything needlessly, but still split the occasional very long lines

I set it to 150. It looks like that value works (for now).

[SirMoM]

I think this is as good as it gets, but if you want to have some things improved let me know.
Thanks @akien-mga for the feedback!

[akien-mga]

Thanks!