-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Remove (already disabled) command handlers related to 'add suppression' menu command. #74266
Conversation
public const int ErrorListSetSeverityInfo = 0x0126; | ||
public const int ErrorListSetSeverityHidden = 0x0127; | ||
public const int ErrorListSetSeverityNone = 0x0128; | ||
public const int ErrorListSetSeverityDefault = 0x0129; |
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.
Command ids no longer used.
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.
nit: comment above could be updated
@@ -228,7 +227,6 @@ protected override async Task LoadComponentsAsync(CancellationToken cancellation | |||
// we need to load it as early as possible since we can have errors from | |||
// package from each language very early | |||
await this.ComponentModel.GetService<VisualStudioSuppressionFixService>().InitializeAsync(this).ConfigureAwait(false); | |||
await this.ComponentModel.GetService<VisualStudioDiagnosticListTableCommandHandler>().InitializeAsync(this, cancellationToken).ConfigureAwait(false); |
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.
no need to initialize since we have no more custom commands here.
} | ||
} | ||
|
||
private DiagnosticData? TryGetSingleSelectedEntry() |
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.
once this was inlined to return null, the rest fell out.
} | ||
|
||
var selectedDiagnostic = TryGetSingleSelectedEntry(); | ||
if (selectedDiagnostic == null) |
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.
when TryGetSingleSelectedEntry returns null, this whole method is a no op. So it doesn't call SetSeverityHandlerAsync, which unwinds everything.
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.
Followups to #74264 and #74265.
We are tracking the request to supply this (through LSP) with #74228. The core code to still do this will exist, but we won't use these command handlers. This PR can also be looked at in case we think there's any interesting logic in the command handler we'd actually want to preserve.