-
Notifications
You must be signed in to change notification settings - Fork 676
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
Show toast when project loading fails #6060
Conversation
@@ -198,6 +199,25 @@ export class RoslynLanguageServer { | |||
); | |||
}); | |||
|
|||
this._languageClient.onNotification(RoslynProtocol.ProjectLoadFailureNotification.type, async (failure) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a bunch of notifications at once are we going to spam the user? Or does VS Code throttle this in some way for us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. Can we kick off an array of projects and 1 collated exception that failed load instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from testing it looks like only 1 shows at once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dibarbet Once that's dismissed though, does the next one appear? Or the other are just ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the others are ignored. I was OK with that because these just point to the logs which will show all the errors.
6e74f30
to
cee0dbb
Compare
@@ -198,6 +199,25 @@ export class RoslynLanguageServer { | |||
); | |||
}); | |||
|
|||
this._languageClient.onNotification(RoslynProtocol.ProjectLoadFailureNotification.type, async (failure) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. Can we kick off an array of projects and 1 collated exception that failed load instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice if LSP supported this automatically!
I left one comment on the Roslyn side wondering if we should be using a prefix for the notification name, which would apply to this PR.
Resolves #6038
Server side - dotnet/roslyn#69424