search

microsoft / language-server-protocol

Defines a common protocol for language servers.
https://microsoft.github.io/language-server-protocol/
Creative Commons Attribution 4.0 International
11.22k stars 799 forks source link

Clarification: diagnostic ranges #257

Open ljw1004 opened 7 years ago

ljw1004 commented 7 years ago

VSCode supports a few diagnostic ranges that aren't documented in the LSP spec:

I think the LSP spec should call out these two cases, so that other clients implementing it will be sure to do the same.

dbaeumer commented 7 years ago

Good catch.

rcjsuen commented 6 years ago

Looks like this has caused 535165 in LSP4E.

mickaelistria commented 6 years ago

I don't think it's necessary to specify anything here. It's fine to get each client dealing with it as they prefer, and the best thing when this occur is to report issues to the LS that produce such ranges to get something more explicit.

floitsch commented 5 years ago

There are at least two clients now that don't deal nicely with these ranges. (intellij and codemirror)

Some language servers thus don't work nicely for all clients. (Admittedly, these servers don't follow the spec, but it should be reasonable to assume that the server is doing ok, when it works with some clients).

Would be nice if the spec got updated.

puremourning commented 5 years ago

What VScode does is irrelevance to the protocol. I think the protocol should stay as is and any server doing any thing else declared nonconformist

Aerijo commented 5 years ago

If a diagnostic's range has start and end position the same, then VSCode will squiggle the word at that position.

I don't like the client interpreting special ranges in general, but this is especially weird. How does the client know the range of a "word"? I know the client might have something like a "word regex" defined, but this is an approximation at best. Isn't the whole point of the language server to understand the language, and handle things like knowing word boundaries, etc.

puremourning commented 5 years ago

it should be reasonable to assume that the server is doing ok, when it works with some clients

I don't agree at all. As a client implementer, what choice do I have but to implement the specification. If VSCode is the "de facto" specification, then why have LSP at all? I mean sure, I get that there is a certain bias towards testing with that client for obvious reasons, but I think server authors should implement the specification and lobby for changes to the specification (or the client) where there is a deviation in the client implementation they test with.

kdvolder commented 5 years ago

I don't like the client interpreting special ranges in general

How would you show a range of length 0? If you underline or highlight something that has no length then the underline will have 0 length and that means you cannot see it.

I don't think the protocol should specify exactly how a client should visualize problem markers. These kinds of visuals depend on the specific client. To me vscode highlighting a word is a reasonable choice on how to visualize a 0 length range. It is better than drawing nothing at all (make it 'invisible'). Other choice could be to try and highlight the character before or after the range instead. Though it may still be hard to see.

Anyhoo... I think there is nothing wrong with leaving this sort of decision up to the client. I wouldn't mind seeing some words added to the spec to encourage/advise clients should try their best to show markers in some way that the user can actually see them, especial for cases where the range is of length 0.

Aerijo commented 5 years ago

How would you show a range of length 0?

However best works for my client.

But it seems you agree with me; this should not be specced by the LSP. How a client wants to show the declared lints is irrelevant to the communication between client and server. If the server really did mean the problem was with the nearest word, then it should have returned that range in the first place. Otherwise, it's just a matter of how the client displays zero width lints.

Sure, the spec should note zero width lints are possible. I also think it should discourage servers from using them unless absolutely necessary though.

kdvolder commented 5 years ago

But it seems you agree with me

Yes, I think so. I didn't think so at first as you seemed to be objecting to vscode deciding to extend the range of a zero-length range to a 'word'. But yes, I agree, this should not be specced. Clients should be free to decide what is the best way to visualize problem markers. And to me this includes choices like what vscode is doing to extend the range of a 0 length range. Other clients should be free to do the same... or not (and then they can find some other way to visualize a 0 length range instead).

ljw1004 commented 3 years ago

There's a legitimate scenario of an underlying third-party language service which reports line-numbers but doesn't report column numbers. What should an LSP adapter do for that language service?

I think LSP should be updated to support a whole-line diagnostic without requiring the LSP adapter to explicitly convey the length of the line in characters. It'd be easiest to do this by just enshrining VSCode's behavior "Number.MAX_VALUE" as a documented part of the LSP spec, and asking other clients to follow.