Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send valid document range for textDocument/format #1177

Open
Waqar144 opened this issue Jul 19, 2024 · 5 comments
Open

Send valid document range for textDocument/format #1177

Waqar144 opened this issue Jul 19, 2024 · 5 comments

Comments

@Waqar144
Copy link

Waqar144 commented Jul 19, 2024

Code editor

Kate

Platform

Linux

Version

No response

What steps will reproduce the bug?

For the textDocument/format request, this server sends an invalid range as reply with the end set to MAX_VALUE instead of the actual end. This breaks some clients such as Kate.

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

Valid document range

What do you see instead?

Invalid document range

Additional information

LSP.Position.create(Number.MAX_VALUE, Number.MAX_VALUE),

IIUC, such special values are against the spec https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#position.

Position in a text document expressed as zero-based line and zero-based character offset. A position is between two characters like an ‘insert’ cursor in an editor. Special values like for example -1 to denote the end of a line are not supported.

@chris-reeves
Copy link
Contributor

Thanks for the report. I'll aim to look into this next week.

@chris-reeves
Copy link
Contributor

I've started to look into this. First off, I get that specifying the end of the current/original document as the end of the range (rather than just picking the largest possible number) would likely fix Kate. Before doing that, I'm keen to understand why this is breaking Kate (for my own edification, if nothing else).

Regarding the spec, I don't think Number.MAX_VALUE would be considered a "special value" in my interpretation of the spec. My reading is simply that there are no magic numbers which mean "end of line" or "end of document". Therefore Number.MAX_VALUE is a valid value (certainly for a sufficiently large document!).

Digging into Kate a little, I can see that the LSPTextEdit returned by the language server should be parsed and validated correctly by parseTextEdit (following this down the call stack I can see that ultimately LSPPosition is mapped to KTextEditor::Cursor and would be considered a valid position according to the docs). So my best guess is that KTextEditor::Document::transformRange or KTextEditor::Document::replaceText is unable to handle a range end that is outwith the current document. Does that sound about right?

Actually, having dug yet further, it looks like KTextEditor::DocumentPrivate::removeText (at least) should be able to handle the end of the range being past the end of the document. Any ideas? What behaviour are you seeing in Kate?

@chris-reeves
Copy link
Contributor

Getting the correct line number to use in the range end should be fairly straightforward (document.lineCount()), but getting the correct character number will be tricker and would need careful thinking and reading of the spec to make sure multi-byte characters and line endings are accounted for correctly.

It would be good to better understand the issues that Kate is having before going down this route.

@Waqar144
Copy link
Author

Waqar144 commented Jul 29, 2024

(Of the top of my head) A range containing 0 -> MAX_VALUE, if invalid, would most likely be converted to an empty range by the MovingRange (think of it as a smart range that automatically adapts to text changes in the document) I think. Thus remove text fails i.e., removes nothing and we end up duplicating the document.

getting the correct character number will be tricker and would need careful thinking and reading of the spec

It would be utf-16 based IIUC second para. Or I would do whatever vscode does to get column number correctly.


Regardless, this issue can actually be solved on the Kate side by doing what VSCode (and nvim?) does which is snap the range to the end of document if the range is bigger and I am actually considering this since there are other servers having this same problem.


Also see microsoft/language-server-protocol#257 for a similar issue.

@Waqar144
Copy link
Author

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

No branches or pull requests

2 participants