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

extensible completion item and symbol kinds #129

Closed
vladdu opened this Issue Nov 17, 2016 · 21 comments

Comments

Projects
None yet
3 participants
@vladdu
Contributor

vladdu commented Nov 17, 2016

The lists of completion item and symbol kinds are hardcoded and match the existing language servers; what should languages that have other kinds do? This is partly a client issue (because it might want to know about these kinds, to provide icons and other UI), but affects the protocol too.

I believe that the ClientCapabilities could be used to expose the supported kinds (whose details for registration would be client-specific)

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Nov 21, 2016

Agree. Two ideas: we could come up with a model of values which are open. And I like the idea that the client lists its available kinds.

@dbaeumer dbaeumer added this to the 3.0 milestone Nov 21, 2016

@felixfbecker

This comment has been minimized.

felixfbecker commented Dec 27, 2016

I think this needs to be handled with great care. If we allow languages to specify arbitrary SymbolKinds, how should the client know what icon is best suited without special casing every language? Defeats the purpose of LSP. I would rather add more SymbolKinds that support as many languages as possible, for example there is no SymbolKind for traits atm.

@vladdu

This comment has been minimized.

Contributor

vladdu commented Dec 27, 2016

I agree that it needs to be handled with care.

If the set of kinds is closed and one needs a new kind, then one needs to propose it, have it accepted as part of the protocol (which will get a new version number?), wait until the client or server supports it and only then implement it. Isn't there a risk that this might be a slow process?

Also, it should be specified what a client should do if it gets an unknown value.

@felixfbecker

This comment has been minimized.

felixfbecker commented Jan 15, 2017

It would only get a version number if it is a breaking change. Adding a new SymbolKind is not a breaking change as a client should just ignore an unknown SymbolKind (display no icon).

@vladdu

This comment has been minimized.

Contributor

vladdu commented Jan 15, 2017

Client needing this might implement extra symbols as private extensions, and when another symbol is accepted it might get the same number. Maybe the protocol could specify a private area where non-standard values can be located without risking collisions?

@felixfbecker

This comment has been minimized.

felixfbecker commented Jan 15, 2017

Just do a proposal to add more symbols. If you add this as a custom extension, only select editors and one language will support this. Defeats the purpose of LSP. We want every editor to be able to show an icon for every symbol in every language, right?

@vladdu

This comment has been minimized.

Contributor

vladdu commented Jan 15, 2017

I'm still getting my head around the "all generic" idea. There are still things that will be extensions to the protocol, so only some servers and clients will recognise them, but I agree that icons might not be one of them. Thanks for taking the time to explain.

@felixfbecker

This comment has been minimized.

felixfbecker commented Jan 15, 2017

The idea is pretty well explained here: http://langserver.org/

@vladdu

This comment has been minimized.

Contributor

vladdu commented Jan 15, 2017

Yes, it's just the "I need X, should the protocol support it or should it be an extension?" part that I'm working on. Some things will be general enough to make it the protocol, some won't.

@felixfbecker

This comment has been minimized.

felixfbecker commented Jan 15, 2017

What exact SymbolKind are you missing?

@vladdu

This comment has been minimized.

Contributor

vladdu commented Jan 15, 2017

"record", "macro", "attribute".

If it were only for displaying icons, these could be mapped to similar concepts, but the client may use textual names and then they should match the language's names.

@felixfbecker

This comment has been minimized.

felixfbecker commented Jan 16, 2017

Not sure what record is, macro sounds like a useful SymbolKind for all languages, isn't attribute the same as property or field? I would encourage you to just propose them. At least in VS Code the symbol kind only influences the icon, all textual information comes through name, description and documentation. If you think a textual description for the type is needed maybe that should be added as a field to SymbolInformation

@vladdu

This comment has been minimized.

Contributor

vladdu commented Jan 16, 2017

Ok, I will do that. (record is like a struct, attribute is more like a pragma) Thanks!

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Feb 8, 2017

I see two options here:

  • we do a careful addition of new constants to ensure that the client will present a good icon and label for it.
  • we extend the protocol so that servers can announce their own constant values. However in this case the server also needs to register a label and an icon for the constant.
@vladdu

This comment has been minimized.

Contributor

vladdu commented Feb 8, 2017

Unfortunately the client might use different icons for different themes, or require multiple sizes or resolutions, for example, so it would be awfully impractical to negotiate with the server the right image to use.

So I think your first suggestion is the only practical one. One addition: since servers might interact with clients that don't understand yet the new values, they could specify in the initialisation a mapping from these new values to the "standard" ones, to be used as a default. Then I think everyone can be happy!

@felixfbecker

This comment has been minimized.

felixfbecker commented Feb 8, 2017

I agree, but I don't think we should let the server provide a map. That would encourage custom items. Let's add new items to the spec and let the client show no icon for unknown values.

@vladdu

This comment has been minimized.

Contributor

vladdu commented Feb 8, 2017

I see your point, but then all unknown symbols would look alike, and what would the label be? "unknown symbol "? Maybe the client could notify the range of symbols it knows about and then the server can choose to map the symbols itself. As long as symbol IDs don't change and aren't removed, it should work just as well.

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Nov 27, 2017

I extended the specification to be able to add more completion item kinds and symbol kinds. I also added some new values.

I still believe that we should try to standardize the set and not make it an open ended list which clients or server then handle by using default values. Therefore I opened

#343
#344

which we should use to collect new kinds which we can then add to the protocol if there is agreement that they should be standardized.

@dbaeumer dbaeumer closed this in 05e3af3 Nov 27, 2017

@vladdu

This comment has been minimized.

Contributor

vladdu commented Nov 27, 2017

Great! Should the default value for the unknown case be specified in the protocol? Maybe "unknown=0"?

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Nov 29, 2017

No, the trick is that a server never will send unknown. It sends well known kinds and the client (if a kind is unknown to him) needs to handle it gracefully. And important for a completion item use the kind provided by the server in a resolve call even if the kind was unknown.

@vladdu

This comment has been minimized.

Contributor

vladdu commented Nov 29, 2017

right, sorry for the confusion, I mixed up the protocol with expectations on the clients.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.