AdaCore / ada_language_server

Server implementing the Microsoft Language Protocol for Ada and SPARK
GNU General Public License v3.0
239 stars 55 forks source link

[Bug]: GPR LS textDocument/hover seems to use CR for EOL #1204

Closed brownts closed 2 months ago

brownts commented 2 months ago

Environment

Bug Summary and Reproducer

When using the GPR LS (i.e., "ada_language_server --language-gpr") with Eglot (Emacs LSP client), I noticed that textDocument/hover responses seem to use "\r" as the EOL marker. Eglot doesn't expect this (as it assumes "\n"), so multi-line hover information might be displayed on a single line containing "^M" characters. I've reported this on the Emacs bug mailing list here. While this seems to be allowed by the LSP specification (at least I couldn't find anything to indicate it wasn't allowed), it also seems that it is unusual for the LS to use "\r" instead of "\n" for this data, at least according to the Eglot author.

I'm trying to get clarification on the reasoning behind the use of "\r" as opposed to "\n" for the EOL in these cases. It appears this is used no matter the native EOL of the platform or the EOL of the applicable file, as I've verified this behavior both on Windows and Linux. Looking at some of the test procedures in the repository (e.g., gpr_lsp/hover/test.json), it appears this behavior is intentional, although the reasoning for this escapes me.

Can you provide an explanation for the choice of "\r"? Would it be possible to use "\n" instead of "\r" in these cases?

Configuration and Logs

[jsonrpc] e[19:19:44.606] <-- textDocument/hover[38]
{"jsonrpc":"2.0","id":38,"result":{"contents":[{"language":"gpr","value":"type
Library_Kinds is (\"relocatable\", \"static\",
\"static-pic\");\rLibrary_Kind : Library_Kinds :=
\"static\";"},"gtkada_shared.gpr:37:04"]}}

Other VS Code Extensions

No response

Additional context

No response

AnthonyLeonardoGracio commented 2 months ago

Hello @brownts,

I think that it's not intentional on our side. We'll change that.

Regards,

AnthonyLeonardoGracio commented 2 months ago

This is now fixed in the development version and will be available in the next extension's release.

Closing the issue.

AnthonyLeonardoGracio commented 2 months ago

Just to be clear: it won't be available in the version released today (25.0.20240915) because the release tag was just done just before, so it will be available in the release right after, which should be more or less published in 1 or 1,5 month approximately.

Regards,