Scony / godot-gdscript-toolkit

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

`gdlint` trailing-whitespace false detection #273

Closed MikeSchulze closed 7 months ago

MikeSchulze commented 7 months ago

At first, thanks for providing this very helpfully tools.

I start to using gdlint and run into some unexpected issue. I use my custom .gdlintrc to lint my scripts and run into a lot of errors

As example script i

#!/usr/bin/env -S godot -s
extends SceneTree

const GdUnitTools := preload("res://addons/gdUnit4/src/core/GdUnitTools.gd")

#warning-ignore-all:return_value_discarded
class CLIRunner extends Node:

    enum {
        READY,
        INIT,
        RUN,
        STOP,
        EXIT
    }

    const DEFAULT_REPORT_COUNT = 20

For the inner class, all lines are indented by one tab, including empty lines. This results into unexpected lint errors.

addons/gdUnit4/bin/GdUnitCmdTool.gd:8: Error: Trailing whitespace(s) (trailing-whitespace)
addons/gdUnit4/bin/GdUnitCmdTool.gd:16: Error: Trailing whitespace(s) (trailing-whitespace)

These are not trailing whitespace, it is leading whitespace.

Scony commented 7 months ago

Actually these are trailing whitespaces, the proper code would be:


extends SceneTree

const GdUnitTools := preload("res://addons/gdUnit4/src/core/GdUnitTools.gd")

#warning-ignore-all:return_value_discarded
class CLIRunner extends Node:

    enum {
        READY,
        INIT,
        RUN,
        STOP,
        EXIT
    }

    const DEFAULT_REPORT_COUNT = 20
MikeSchulze commented 7 months ago

Hi @Scony, the code has leading whitespaces and not trailing this is a difference.

Scony commented 7 months ago

@MikeSchulze you're right except for lines 8 and 16 - those are blank lines with whitespaces i.e. trailing whitespaces.

MikeSchulze commented 7 months ago

Sorry, but I don't understand how there can be a blank line with tabs at the end? The reading is still from left to right, isn't it? The tabs are at the beginning, if I put a regex with ^(\t) over them, they are found at the beginning. With ^(\t+)$ valid blank lines should also be recognized. I prefer to format my code so that the indentations are consistent throughout the code. However, this is currently not possible if these lines are recognized as errors.

Since CLIRunner is an inner class, a tabulator is always required on each line, this should also apply to "empty" lines.

It would be nice to have an extra flag to control this kind of "error". e.g. trailing-whitespace-empty-line: ignore

Scony commented 7 months ago

Well, my point is - ^\t+$ lines are considered to be invalid by gdlint. Such blank lines with whitespaces are a special case of trailing whitespaces where entire lines (all whitespaces) are trailing - i.e. not necessary. Normally, formatters remove such redundancy and linters complain about it. If you're using git, you may notice it highlights such cases. Therefore the behavior of gdlint mentioned in this ticket is correct. I'm not sure why you'd like to have such indented blank lines, but anyway, gdlint does not and will not support that. Sorry.