Implement hcl parse diagnostics #269
Conversation
|
|
||
| func NewNotifier(sessCtx context.Context) *Notifier { | ||
| hclDocs := make(chan documentContext, 10) | ||
| go hclDiags(hclDocs) |
There was a problem hiding this comment.
Is there a reason we need the asynchronicity?
I mean each request is already being processed in a dedicated goroutine and parallelism is controlled in the jRPC library and that's limited to 1 for ordering reasons, so we'd never actually process more than 1 at a time anyway and even if for some reason we do in the future, then I reckon the performance/parallelism would be handled by the jRPC library on handler level?
There was a problem hiding this comment.
Are push requests limited by the parallelism? Seems like this is just pushing each chan value.
There was a problem hiding this comment.
The idea is we have a diagnostics worker thread that takes in document requests and pushes diags to the client as soon as possible, but it does not ever block or add milliseconds to the didOpen and didChange handlers (and upcoming didSave for validation).
There was a problem hiding this comment.
Totally nitpick: I would probably pick a different name for lsp/convert.go, just because technically every logic in that package is "converting" in some form or shape. Perhaps it can be diagnostics.go for the severity stuff and range.go for the other two functions.
This PR should be followed up on to try and deliver on that, wanted to keep it simple win for now
Agreed - can you please create an issue for this, describing roughly what needs to be done, just so we don't forget?
|
We were testing this yesterday and it looked like the squiggles may have an off by one display issue but unsure. Just wanted to note it here for double checking. |
I will see if I can quickly resolve this or if it's coming from the hcl parser UPDATE: quickly verified there are no off by one errors, some syntax errors just result in some not super accurate diags from the hcl parser. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
ghost
locked as resolved and limited conversation to collaborators
Diagnostic service asynchronously parses opened and changed documents, and pushes diagnostics to the client. Async model prevents
didOpen,didChange, or future handlers from blocking/slowing down during more expensive validation.The protocol specifies that all files of the project should be diagnosed at once:
This PR should be followed up on to try and deliver on that, wanted to keep it simple win for now