Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

Godot-gdscript-toolkit formatter and linter support #827

Merged
merged 8 commits into from
Apr 29, 2022
Merged

Godot-gdscript-toolkit formatter and linter support #827

merged 8 commits into from
Apr 29, 2022

Conversation

Arrow-x
Copy link
Contributor

@Arrow-x Arrow-x commented Apr 25, 2022

just added the formatter gdformat from https://github.com/Scony/godot-gdscript-toolkit
it doesn't work with lua vim.lsp.buf.formatting_sync() I don't know why...help

@Arrow-x Arrow-x mentioned this pull request Apr 25, 2022
1 task
@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 25, 2022

lsp
this is what Lspinfo says

@jose-elias-alvarez
Copy link
Owner

Thanks for putting this in! You can try enabling debug logging and see if the output from the formatter matches what you expect. Otherwise, I'll have a chance to test this out myself later this week.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 25, 2022

so this is what it gave me

[TRACE Mon 25 Apr 2022 09:03:36 PM CET] .../site/pack/packer/start/null-ls.nvim/lua/null-ls/rpc.lua:118: received LSP request for method textDocument/formatting
[TRACE Mon 25 Apr 2022 09:03:36 PM CET] ...ack/packer/start/null-ls.nvim/lua/null-ls/generators.lua:21: running generators for method NULL_LS_FORMATTING
[DEBUG Mon 25 Apr 2022 09:03:36 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:344: spawning command "gdformat" at /home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System with args {}
[TRACE Mon 25 Apr 2022 09:03:36 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:217: error output: Usage:
  gdformat <path>... [options]

[TRACE Mon 25 Apr 2022 09:03:36 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:218: output: nil
[TRACE Mon 25 Apr 2022 09:03:36 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:222: ignoring stderr due to generator options

gdformat is very simple it is just gdformat path/to/file

@jose-elias-alvarez
Copy link
Owner

Are you sure it supports stdin? I don't see it in any of the examples on the repo. In that case, you'd want to pass the filename as an argument and also use to_temp_file - see here for an example.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 25, 2022

it does support stdin it needed the arg "-", I did that and the formatter works, now I will attempt to make the linter work or should put that in another PR?

@jose-elias-alvarez
Copy link
Owner

Either way works! I'll mention that diagnostic sources can be significantly trickier depending on the output format.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 25, 2022

I will give it a try!
(I don't know what the codestyle checker want for me (I will just install stylelua on my end I think))

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 26, 2022

so the null-ls debug catches the output from gdlint and display it
...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:217: error output:
but then it nil it?

[WARN  Tue 26 Apr 2022 02:05:06 PM CET] ...ack/packer/start/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:232: error in generator output: ```

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 26, 2022

this is the full log

[TRACE Tue 26 Apr 2022 04:07:49 PM CET] ...te/pack/packer/start/null-ls.nvim/lua/null-ls/client.lua:92: starting null-ls client
[TRACE Tue 26 Apr 2022 04:07:49 PM CET] .../site/pack/packer/start/null-ls.nvim/lua/null-ls/rpc.lua:118: received LSP request for method initialize
[TRACE Tue 26 Apr 2022 04:07:49 PM CET] .../site/pack/packer/start/null-ls.nvim/lua/null-ls/rpc.lua:143: received LSP notification for method initialized
[TRACE Tue 26 Apr 2022 04:07:49 PM CET] .../site/pack/packer/start/null-ls.nvim/lua/null-ls/rpc.lua:143: received LSP notification for method textDocument/didOpen
[TRACE Tue 26 Apr 2022 04:07:49 PM CET] ...ack/packer/start/null-ls.nvim/lua/null-ls/generators.lua:21: running generators for method NULL_LS_DIAGNOSTICS_ON_OPEN
[DEBUG Tue 26 Apr 2022 04:07:49 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:344: spawning command "gdlint" at /home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System with args { "/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd" }
[TRACE Tue 26 Apr 2022 04:07:50 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:217: error output: /home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:47: Error: Max allowed line length (100) exceeded (max-line-length)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:45: Error: Function name "_on_FileDialog_Closed" is not valid (function-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:10: Error: Function-scope variable name "_tab" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:61: Error: Function-scope variable name "_editor" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:75: Error: Function-scope variable name "_plus" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:95: Error: Function-scope variable name "_warning" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:4: Error: Class-scope variable name "editorUI" is not valid (class-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:5: Error: Definition out of order in global scope (class-definitions-order)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:6: Error: Definition out of order in global scope (class-definitions-order)
Failure: 9 problems found

[TRACE Tue 26 Apr 2022 04:07:50 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:218: output: nil
[WARN  Tue 26 Apr 2022 04:07:50 PM CET] ...ack/packer/start/null-ls.nvim/lua/null-ls/generators.lua:78: failed to run generator: ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:232: error in generator output: /home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:47: Error: Max allowed line length (100) exceeded (max-line-length)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:45: Error: Function name "_on_FileDialog_Closed" is not valid (function-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:10: Error: Function-scope variable name "_tab" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:61: Error: Function-scope variable name "_editor" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:75: Error: Function-scope variable name "_plus" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:95: Error: Function-scope variable name "_warning" is not valid (function-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:4: Error: Class-scope variable name "editorUI" is not valid (class-variable-name)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:5: Error: Definition out of order in global scope (class-definitions-order)
/home/arrowx/File/Projects/Godot/Mushroom-Dialogs-System/DialogSystem/DialogManager/Editor/Editor.gd:6: Error: Definition out of order in global scope (class-definitions-order)
Failure: 9 problems found

@jose-elias-alvarez
Copy link
Owner

jose-elias-alvarez commented Apr 27, 2022

Looks like it's outputting to stderr. You can use from_stderr = true to get the output. After that, it's a matter of parsing each line to get the position and message. I recommend taking a look at our other built-ins for diagnostics to see how they do it.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 27, 2022

I got it to work!
linter2022-04-27_04-10
but I have some problems, it doesn't update with the changes of the file.

@jose-elias-alvarez
Copy link
Owner

jose-elias-alvarez commented Apr 28, 2022

That indicates that it's not accepting input from stdin. Assuming it accepts the same argument structure as the formatter, it should be an easy fix, but otherwise we can use a temp file here, too.

Edit: also, to clear the CI failures, you'll just want to run the files through StyLua, either through the command line or via the null-ls built-in.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 28, 2022

Well it works, it indeed needs a temp file (doesn't accept from stdin nor output one) and it worked as expected! Sadly, the linter doesn't report columns
What should I do now ??

@Arrow-x Arrow-x changed the title Godot-gdscript-toolkit formatter support Godot-gdscript-toolkit formatter and linter support Apr 28, 2022
@jose-elias-alvarez
Copy link
Owner

jose-elias-alvarez commented Apr 28, 2022

Two options:

  1. You can try to parse the diagnostic and find the matching column in the line it applies to (for example, this should work for the diagnostics that reference a specific variable name)
  2. You can just leave it off and have them apply to the whole line (this is totally fine, and we have other sources that do the same, since the linter doesn't provide the info)

Either way, judging from the output you posted, I think we'll need #2 as a fallback.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 28, 2022

If there are other linters that do the same, I will leave it as is.
and about #2 ….what? I don't understand what I have to do in that front.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 28, 2022

[TRACE Thu 28 Apr 2022 11:56:09 PM CET] .../site/pack/packer/start/null-ls.nvim/lua/null-ls/rpc.lua:143: received LSP notification for method textDocument/didClose
[TRACE Thu 28 Apr 2022 11:56:13 PM CET] .../site/pack/packer/start/null-ls.nvim/lua/null-ls/rpc.lua:143: received LSP notification for method textDocument/didOpen
[TRACE Thu 28 Apr 2022 11:56:13 PM CET] ...ack/packer/start/null-ls.nvim/lua/null-ls/generators.lua:21: running generators for method NULL_LS_DIAGNOSTICS_ON_OPEN
[DEBUG Thu 28 Apr 2022 11:56:13 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:344: spawning command "gdlint" at /home/arrowx/File/Projects/Godot/Cont with args { "/tmp/null-ls_LTC9wr.gd" }
[TRACE Thu 28 Apr 2022 11:56:13 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:217: error output: /tmp/null-ls_LTC9wr.gd:6: Error: Definition out of order in global scope (class-definitions-order)
Failure: 1 problem found

[TRACE Thu 28 Apr 2022 11:56:13 PM CET] ...t/null-ls.nvim/lua/null-ls/helpers/generator_factory.lua:218: output: nil
[TRACE Thu 28 Apr 2022 11:56:13 PM CET] ...ck/packer/start/null-ls.nvim/lua/null-ls/diagnostics.lua:169: received diagnostics from source 5
[TRACE Thu 28 Apr 2022 11:56:13 PM CET] ...ck/packer/start/null-ls.nvim/lua/null-ls/diagnostics.lua:170: { {
    col = 0,
    end_col = 0,
    end_lnum = 6,
    lnum = 5,
    message = "Error: Definition out of order in global scope (class-definitions-order)",
    row = "6",
    severity = 1,
    source = "gdlint"
  } }

This is a new output with temp files and no stdin

@jose-elias-alvarez
Copy link
Owner

Sorry, I should have been clearer - if you leave it as-is, it'll just default to 0, meaning it applies to the entire line (which is totally fine).

If everything is working, this looks good to me, but let me know when you think it's ready to merge.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented Apr 29, 2022

Oh, ok! I think it's ready to merge, I tested it a bit more, and it's seem fine.

@jose-elias-alvarez
Copy link
Owner

Awesome, thanks for working on this!

@jose-elias-alvarez jose-elias-alvarez merged commit 1e39119 into jose-elias-alvarez:main Apr 29, 2022
@sassanh
Copy link

sassanh commented May 11, 2023

gdlint is not working with null-ls for me. Running gdlint I see errors in this format:

> gdlint Globals.gd
Globals.gd:

var cap_mesh: Dictionary<
                        ^

Unexpected token Token('LESSTHAN', '<') at line 4, column 25.
Expected one of: 
        * SEMICOLON
        * $END
        * EQUAL
        * COLON
        * _NL
Failure: 1 problem found

Did they change the output format or am I missing a config? Currently null-ls reports no errors via the source gdlint, it shows errors reported by gdscript lsp.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented May 11, 2023

@sassanh it works fine on my end?
Godot 3
image
Godot 4
image

it works with both branches of gdtoolkit make sure you are installing the correct one for the version of godot you are using
activate it using

require("null-ls").setup({
    sources = {
        require("null-ls").builtins.diagnostics.gdlint
    }
})

@sassanh
Copy link

sassanh commented May 11, 2023

Thanks for the quick reply, I do have that source in my null-ls setup:

      null_ls.setup {
        on_attach = On_attach,
        sources = {
          null_ls.builtins.code_actions.gitsigns,
          null_ls.builtins.formatting.gdformat,
          null_ls.builtins.diagnostics.gdlint,
        },
      }

gdformat works well when I write the file, but gdlint doesn't work, gdlint command line on the hand works as expected, it is just not working with null-ls. It even shows as active in NullLsInfo:

	* name: gdlint
	* filetypes: gdscript
	* methods: diagnostics

If it is working fine for you then there should be something wrong in my setup.

@sassanh
Copy link

sassanh commented May 11, 2023

I'm using gdtoolkit 4.0.1 from pypi, what do you mean by "both branches" of gdtoolkit? Is gdlint generating errors like this for you?

gdlint Globals.gd
Globals.gd:

var cap_mesh: Dictionary<
^

Unexpected token Token('LESSTHAN', '<') at line 4, column 25.
Expected one of:
* SEMICOLON
* $END
* EQUAL
* COLON
* _NL
Failure: 1 problem found

@sassanh
Copy link

sassanh commented May 11, 2023

The output generated by my gdlint binary doesn't seem to match with what these lines expect:
https://github.com/jose-elias-alvarez/null-ls.nvim/pull/827/files#diff-9e4e64a2730fb6ef499cc098bcbcb34cdf0fa89d66bcedf658af4cf820b7af06R24

        on_output = h.diagnostics.from_patterns({
            {
                pattern = [[:(%d+): (.*)]],
                groups = { "row", "message" },
            },
        }),

So I guess your gdlint is generating errors in a different format.

@sassanh
Copy link

sassanh commented May 11, 2023

I think I understand what's happening now, gdlint raises errors and reports them in a different format when the code has certain syntax errors. And while defining functions inside functions works with Godot, seems like gdlint can't handle it.

So there is no problem with the way we are setting up gdlint for null-ls.

@Arrow-x
Copy link
Contributor Author

Arrow-x commented May 11, 2023

there are two branches for gdtoolkit
for Godot 4
pip3 install "gdtoolkit==4.*"
and for Godot 3
pip3 install "gdtoolkit==3.*"
I tested both on my end they both work fine with the example that you showed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants