Schottkyc137 / ginko

A device-tree source parser, analyzer and language server.
MIT License
9 stars 4 forks source link

Provide content-length property #12

Closed jclsn closed 1 month ago

jclsn commented 2 months ago

Hi, thanks for developing a Device Tree language server! I have been waiting for this. There is some information missing that CoC needs to function with the language server. I had a similar issue with the bitbake-language-server and it needs to be fixed from the language server side

[coc.nvim] Connection to server ginko_ls is erroring, Header must provide a Content-Length property.
[coc.nvim] Connection to server ginko_ls is erroring, Header must provide a Content-Length property.
{"{\"jsonrpc\"":"\"2.0\",\"method\":\"textDocument/publishDiagnostics\",\"params\":{\"diagnostics\":[{\"message\":\"Expected string\",\"range\":{\"end\":{\"character\":10,\"line\":1},\"start\":{\"character\":9,\"line\":1}},\"severity\":1}],\"uri\":\"file:///home/jan/Workspace/Yocto/build/workspace/sources/linux/arch/arm64/boot/dts/freescale/fsl-ls1028a.dts\"}}\u001b[2m2024-06-24T12:33:13.361769Z\u001b[0m \u001b[32m INFO\u001b[0m \u001b[2mtower_lsp::service::layers\u001b[0m\u001b[2m:\u001b[0m shutdown request receiv
ed, shutting down\nContent-Length: 38"}. Shutting down server.
Schottkyc137 commented 2 months ago

Hi, thanks for your interest in the project! I think this isn't related to not providing the Content-length property, but instead there seems to be something printed to stdout where it shouldn't be. In the log you provided, there is an ominous \u001b[2m2024-06-24T12:33:13.361769Z\u001b[0m \u001b[32m INFO\u001b[0m \u001b[2mtower_lsp::service::layers\u001b[0m\u001b[2m:\u001b[0m shutdown request received, shutting down\n appearing in the message. After that, the Content-Length property appears normally. I'm wondering whether this is related to the usage of coc.nvim, as I cannot observe the error in VSCode (and I think people have used this successfully with Mason, but I cannot confirm that). What seems odd to me is that there seems to be a shutdown request while receiving the textDocument/publishDiagnostics method. Do you know what the solution for bitbake-language-server was?

jclsn commented 2 months ago

Afaik the bitbake-language-server did indeed not provide a content-length property. I don't know about the specifics though. If you are certain that it is provided, there could be an issue elsewhere. I will notify the developers of CoC about this. They are really active and helpful

Schottkyc137 commented 2 months ago

I am not 100% certain, however from my perspective, this issue could be on either side. The only thing I'm observing is that there actually is a Content-Length property provided in the JSON output, however inter-mangled with some console output from the language server. This could be my problem, but this could also be an issue at coc.nvim, because the server is being shut down too early or similar.

fannheyward commented 2 months ago

coc.nvim uses vscode-jsonrpc to communicate with language server, the server should response with content-length header https://github.com/microsoft/vscode-languageserver-node/blob/33cbe5c87032cb53cf30c2b90f29d22a45ee6800/jsonrpc/src/common/messageReader.ts#L223-L227

Schottkyc137 commented 2 months ago

I have finally found the time to try things out myself. I got the following output:

[coc.nvim] Connection to server dts is erroring, Header must provide a Content-Length property.
{"\u001b[2m2024-06-30t17":"26:58.978499Z\u001b[0m \u001b[33m WARN\u001b[0m \u001b[2mtower_lsp\u001b[0m\u001b[2m:\u001b[0m Got a workspace/didChangeConfiguration notification, but it is not implemented\nContent-Leng
[coc.nvim] Connection to server dts is erroring, Header must provide a Content-Length property.
{"{\"jsonrpc\"":"\"2.0\",\"method\":\"textDocument/publishDiagnostics\",\"params\":{\"diagnostics\":[{\"code\":\"expected\",\"message\":\"Expected one of /dts-v1/, /memreserve/, /include/, /delete-node/, /omit-if-n
o-ref/, '/', reference\",\"range\":{\"end\":{\"character\":1,\"line\":0},\"start\":{\"character\":0,\"line\":0}},\"severity\":1,\"source\":\"ginko_ls\"}],\"uri\":\"file:///Users/lukasscheller/.local/share/nvim/site
/pack/x.dts\"}}\u001b[2m2024-06-30T17:26:58.984067Z\u001b[0m \u001b[32m INFO\u001b[0m \u001b[2mtower_lsp::service::layers\u001b[0m\u001b[2m:\u001b[0m shutdown request received, shutting down\nContent-Length: 38"}. 
Shutting down server.

Did you also see the first error message? If so then it seems that the client requests the workspace/didChangeConfiguration which I did not implement explicitly. Implementing this message results in the server being operational, so this would be a quick fix.

jclsn commented 2 months ago

I just saw the error message about posted above

Schottkyc137 commented 2 months ago

So to conclude, I think that there is a way to quick fix the issue. However, I would like to find a more robust solution so I will investigate a bit further. If I don't find a good solution within the coming weeks, I will just roll out the quick fix.

jclsn commented 2 months ago

I am not in a hurry. Do your thing! :)

Schottkyc137 commented 1 month ago

Fixed in the latest master. Will be rolled out with the next update

jclsn commented 1 month ago

Thanks, works well! :)

I created an AUR package btw, which builds directly from source

https://aur.archlinux.org/packages/ginko-git

jclsn commented 1 month ago

Ah well, outline via :CocOutline or goto definitions doesn't work it seems

Schottkyc137 commented 1 month ago

Ah well, outline via :CocOutline or goto definitions doesn't work it seems

Can you elaborate or produce a minimal reproducible example? On my side, :CocOutline and go to definition works. What I used:

/dts-v1/;

/ {
    smth_label: smth {};

    smth_else {
        prop=<&smth_label>;
    };
};

and :CocOutline produces:

OUTLINE Category
- m / 3
    m smth 4
  + m smth_else 6

When placing the cursor over the &smth_label reference and calling call CocActionAsync('jumpDefinition') in the neovim console, the cursor jumps to smth.

Schottkyc137 commented 1 month ago

Thanks, works well! :)

I created an AUR package btw, which builds directly from source

https://aur.archlinux.org/packages/ginko-git

Thanks a lot! If you want to, you can open a PR with installation instructions in the readme

jclsn commented 1 month ago

It seems to only have problems with .dtsi files. e.g.

https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/actions/s700.dtsi

Ah seems to be due to the C includes. You mentioned that in the README.

Adding install instructions for AUR packages is out of the scope of the README. Every Arch user knows how anyway. It is just a convenient way to compile the application from source.

Schottkyc137 commented 1 month ago

It seems to only have problems with .dtsi files. e.g.

https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/actions/s700.dtsi

Ah seems to be due to the C includes. You mentioned that in the README.

Adding install instructions for AUR packages is out of the scope of the README. Every Arch user knows how anyway. It is just a convenient way to compile the application from source.

Unfortunately, this is known and expected at the moment. These files contain C-style include directives (i.e., #include <some/path>) which ginko does not yet support. I know that this is a big pain point, because 99% of real-world device-trees contain these directives, so naturally a language server is only really useful if it also supports these directives. I'm working on it, but as I am maintainer of a different (and bigger) open-source project and have a full-time job as well, my time is limited. I am making progress, but this will be very slow (so don't expect anything any time soon).

jclsn commented 1 month ago

It's fine :) I am grateful for anything that helps when it comes to device trees, as I've always had problems understanding them. I know it now and can just delete the lines as long as I am editing the file.

Mabye just make the parser ignore these lines for now, so the file is parsed at least!