lsp: Update contents before diagnosticRequestFile #1100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
charlieegan3
commented
Sep 11, 2024
| @@ -115,9 +115,8 @@ type LanguageServer struct { | |||
|
|
|||
| // fileUpdateEvent is sent to a channel when an update is required for a file. | |||
| type fileUpdateEvent struct { | |||
| Reason string | |||
| URI string | |||
| Content string | |||
There was a problem hiding this comment.
This has been removed so it's no longer possible to send fileUpdateEvents with no content, this reduces the responsibility on the diagnostics worker. Now it can just update parses and send diagnostics.
charlieegan3
commented
Sep 11, 2024
| @@ -212,9 +211,11 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { | |||
| case <-ctx.Done(): | |||
| return | |||
| case evt := <-l.diagnosticRequestFile: | |||
| err := l.processTextContentUpdate(ctx, evt.URI, evt.Content) | |||
| // updateParse will not return an error when the parsing failed, | |||
There was a problem hiding this comment.
processTextContentUpdate has been removed and inlined here.
charlieegan3
commented
Sep 11, 2024
| @@ -260,9 +265,42 @@ func (l *LanguageServer) StartHoverWorker(ctx context.Context) { | |||
| case <-ctx.Done(): | |||
| return | |||
| case evt := <-l.builtinsPositionFile: | |||
| err := l.processHoverContentUpdate(ctx, evt.URI, evt.Content) | |||
| fileURI := evt.URI | |||
There was a problem hiding this comment.
processHoverContentUpdate has also been inlined here.
anderseknert
approved these changes
Sep 11, 2024
charlieegan3
commented
Sep 11, 2024
| Reason: "internal/templateNewFile", | ||
| URI: fileURI, | ||
| Content: newContents, | ||
| Reason: "internal/templateNewFile", |
There was a problem hiding this comment.
The actual issue this fixes was triggered by
Where an empty update was sent causing the cache's state to be set to blank for files when they were detected as new, despite them first having their contents loaded from disk.
charlieegan3
force-pushed
the
correct-empty-updates
branch
from
a65ce07 to
1a3d2f0
Compare
One of the main causes for the 'empty module' error stemmed from the sending of diagnosticRequestFile events with no content. One solution would have been to fetch the content when processing events on the diagnosticRequestFile channel, however it felt like the publisher had more knowledge of how to do this correctly and processing diagnosticRequestFile was getting unwieldy. I have removed the content from diagnosticRequestFile, and these events will just update the parse and send diagnostics. This should be less surprising. Signed-off-by: Charlie Egan <charlie@styra.com>
charlieegan3
force-pushed
the
correct-empty-updates
branch
from
1a3d2f0 to
424b17a
Compare
srenatus
pushed a commit
to srenatus/regal
that referenced
this pull request
Oct 1, 2024
One of the main causes for the 'empty module' error stemmed from the sending of diagnosticRequestFile events with no content. One solution would have been to fetch the content when processing events on the diagnosticRequestFile channel, however it felt like the publisher had more knowledge of how to do this correctly and processing diagnosticRequestFile was getting unwieldy. I have removed the content from diagnosticRequestFile, and these events will just update the parse and send diagnostics. This should be less surprising. Signed-off-by: Charlie Egan <charlie@styra.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the main causes for the 'empty module' error stemmed from the sending of diagnosticRequestFile events with no content.
One solution would have been to fetch the content when processing events on the diagnosticRequestFile channel, however it felt like the publisher had more knowledge of how to do this correctly and processing diagnosticRequestFile was getting unwieldy.
I have removed the content from diagnosticRequestFile, and these events will just update the parse and send diagnostics. This should be less surprising.