https://github.com/HighCommander4 commented:

Thanks for the patch!

I like the idea of having the server announce support for a list of languages, 
that seems nice and general and extensible.

A couple of thoughts:

 1. Since the key we're adding to the server capabilities is a non-standard 
extension, I'm a bit uneasy about giving it a general name like `"languages"`. 
If the LSP were to add a key with that name but a different format or 
interpretation, we could run into trouble. Can we name it `"clangdLanguages"` 
instead, thereby staying within our own (improvised) "namespace"?
 2. Given that the plan described by Sam in [this 
comment](https://github.com/clangd/vscode-clangd/pull/392#issuecomment-1285902875)
 is to enable vscode-clangd support for HLSL by default once clang's support 
for HLSL is a good enough state, what do you think about the following 
arrangement:
     * don't include `"hlsl"` in `"clangdLanguages"` yet
     * add `"hlsl"` to `"clangdLanguages"` once the "good enough" threshold is 
reached
     * on the client side, add HLSL to the document selector if the server 
announces support for `"hlsl"` in `"clangdLanguages"`, **or** a client-side 
preference (which we could name "experimental", e.g. 
`"clangd.experimentalHLSLSupport"`) is enabled?
     
     This way:
      * early adopters can set `"clangd.experimentalHLSLSupport"` with an 
up-to-date client paired even with a clangd 17 or older server, and get the 
corresponding level of support
     * once the implementation quality is good enough, the server can start 
announcing the support and all clients (not just early adopters who set the 
pref) will have it by default with newer server versions

https://github.com/llvm/llvm-project/pull/75633
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to