Skip to content

Write-Warning instead of returning artificial DiagnosticRecord when type parsing errors occurs since those DiagnosticRecords cannot be suppressed. #1126

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

Conversation

bergmeister
Copy link
Collaborator

@bergmeister bergmeister commented Jan 15, 2019

PR Summary

Fixes #1041 as per title and enhances message.
I tried looking into being able to support suppressions for a DiagnosticRecord but this did not seem to be possible or very complicated. Since TypeNotFound is not a rule, I think it was confusing anyway to return an object. Consumers like PowerShellEditorServices should check their warning streams to propagate the information to their users. cc @TylerLeonhardt @rjmholt
The failing tests on Ubuntu happen on development as well and seem to be due to an image update (again)

PR Checklist

…ype parsing errors occurs since those DiagnosticRecords cannot be suppressed.
@bergmeister bergmeister changed the title Write-Warning instead of returning artificial DiagnosticRecord when type parsing errors occurs since those DiagnosticRecords cannot be suppressed. WIP Write-Warning instead of returning artificial DiagnosticRecord when type parsing errors occurs since those DiagnosticRecords cannot be suppressed. Jan 15, 2019
@bergmeister bergmeister changed the title WIP Write-Warning instead of returning artificial DiagnosticRecord when type parsing errors occurs since those DiagnosticRecords cannot be suppressed. Write-Warning instead of returning artificial DiagnosticRecord when type parsing errors occurs since those DiagnosticRecords cannot be suppressed. Jan 15, 2019
Copy link
Contributor

@JamesWTruher JamesWTruher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this isn't the right approach here. We could create different types of diagnostic records, or create some way to suppress these, but these shouldn't be taking these out of the result stream. They're important, and by putting them in the warning stream, they're completely lost.

@bergmeister
Copy link
Collaborator Author

bergmeister commented Jan 16, 2019

@JamesWTruher I tried looking into a way of suppressing them at first but struggled to do so due to the suppression logic being so complex, maybe you can give it a try please?

@bergmeister
Copy link
Collaborator Author

Will close this in favour of #1130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeNotFound suppressions
2 participants