-
Notifications
You must be signed in to change notification settings - Fork 24
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
No semantic tokens returned for a function call #431
Comments
Hi @rcjsuen, I believe we only add semantic tokens for known function calls. The return &decoder.PathContext{
Schema: bodySchema,
Files: pr.Files,
Functions: map[string]schema.FunctionSignature{
"lower": {
Params: []function.Parameter{
{
Name: "str",
Type: cty.String,
},
},
ReturnType: cty.String,
Description: "`lower` converts all cased letters in the given string to lowercase.",
},
},
}, nil Result
I hope this helps! |
That is very surprising. The parser knows what a function looks like so it is not clear to me why the name of the functions must be registered first? 🤔 |
Considering one of the main roles of this library is to build language servers for HCL-based languages it also means it provides functionality relevant to that HCL-based language, rather than HCL.
The problem "what looks like a function" is valid but it is solved by simpler syntax grammars, such as TextMate - see https://github.com/hashicorp/syntax Most IDEs rely on TextMate or comparable (simpler) grammars (as opposed to parsers) to do the same job. As you rightly said the parser knows what a function looks like but on its own (without schema) doesn't know if it is in fact valid function, or even function call (as opposed to type constraint). HCL has an (opt in) concept of type constraint that make use of function calls - see https://developer.hashicorp.com/terraform/language/expressions/type-constraints#structural-types variable "example" {
type = map(string)
} and it is exactly the role of semantic highlighting to be able to tell the difference between the two. All that said, I guess there could be some opt-in behaviour where the tokens returned don't take valid function names into account. This would only be relevant in a context where there is no better highlighting mechanism mentioned above and semantic highlighting is the only way to highlight AND the highlighting is being done in the context of plain HCL (e.g. https://marketplace.visualstudio.com/items?itemName=HashiCorp.HCL) rather than the HCL-based language like Terraform. @rcjsuen Have you come across such a context? |
Not sure if this answers your question but ultimately I want the language server to colour everything so that the plug-in into the language client is as simplest as possible. |
Personally (bearing in mind I no longer work on the team that maintains this library) I would not be entirely opposed to colouring everything, but it would certainly have to be opt-in behaviour as it would only make sense in the context I described above. It would probably imply maintenance of two highlighting/tokenisation modes though, so I'm not sure the maintenance cost vs value ratio would be very favourable - hence my question about use cases. Which is to say - if you are making a new LS for an HCL-based language, then you should be able to leverage the grammars I linked above - or more specifically this generic HCL one. Presumably you will need a dedicated language client developed anyway, even if it's a lightweight one, to ensure the language IDs are recognised correctly and passed down to the server - so such a client may as well do the basic highlighting using that grammar (doing exactly what you asked for, i.e. highlight what looks like a function) and let the server do just "enrichment" on top of that. We left some guidance in the Readme describing how to use the grammars. I'd specifically point out these two parts considering your questions: |
This HCL file seems pretty straightforward to me but I don't have any semantic tokens for functions. Is this intentional?
The text was updated successfully, but these errors were encountered: