Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upActionabilize errors from incorrect settings files #1263
Conversation
|
The first commit adds only research comments, and the last commit reverts them. However, I have chosen to leave the commits in for documentary purposes in the history. In particular, I imagine that further work either refactoring or extending this code region might benefit from them. |
|
First of all thanks for making the effort and improvement. I agree that throwing a terminating error here is the right decision. One minor comment but hold off from addressing it at the moment, I will add more reviewers so that they can decide as well. But otherwise, it looks good to me. In the future we might enhance it to emit DiagnosticRecords of type ParserError in lower levels so that people get highlighting in VS Code but your change to improve the command line experience is good as a first improvement.
|
|
I think @JamesWTruher was working on something in this space as well. Ideally this should actually throw an But I think this is a step in the right direction. |
|
@bergmeister agreed re integration for other contexts; ideally rather than a parse error the error should be something PSES can catch and then display as a notification. We should discuss what the best design is there, especially in terms of revising the hosting model (@JamesWTruher) |
|
Thanks, looks good to me, just one minor cleanup nit |
| @@ -8,6 +8,7 @@ | |||
| using System.Collections.ObjectModel; | |||
| using System.Diagnostics.CodeAnalysis; | |||
| using System.Globalization; | |||
| using System.IO; | |||
This comment has been minimized.
This comment has been minimized.
bergmeister
Jun 22, 2019
Collaborator
Nit: With the last commit, this using statement is not needed any more.
This comment has been minimized.
This comment has been minimized.
travis-c-lagrone
Jun 23, 2019
Author
Contributor
using System.IO; does appear to be required for pre-existing unqualified references to the type System.IO.InvalidDataException.
This comment has been minimized.
This comment has been minimized.
bergmeister
Jun 29, 2019
Collaborator
Not any more after the last commit that removed this type. I pushed a last commit to fix this and do some minor cleanup and make the format consistent to the solution and merge then after this.
… settings file load invocation
…nvoke-ScriptAnalyzer -Settings' results in an exception
…ScriptAnalyzer -Settings` results in an exception
…processing the argument of `Invoke-ScriptAnalyzer -Settings` results in an exception
1fc5c77
to
88ab1ec
…name and make error record arguments consistent with usage in other parts of the solution.
b407e8e
into
PowerShell:master

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

travis-c-lagrone commentedJun 18, 2019
•
edited by bergmeister
Fixes #1053 "Errors from incorrect settings files are not actionable".