-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
feat: enable jumping to "not formatted" files from VisualStudio error list #1517
base: main
Are you sure you want to change the base?
Conversation
… list - adds --msbuild-format parameter to format error message according to standard error/warning format of MSBuild. See https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-diagnostic-format-for-tasks?view=vs-2022
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.
I'm happy to see that this cleans up the msbuild target. The main change is making the option an enum so it'll be easier to add #1398 later.
new Option( | ||
["--msbuild-format"], | ||
"Formats messages in standard error/warning format for MSBuild." | ||
), |
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.
There have been requests for other log formats. I'm thinking this should be --log-format
and accept a string/enum. Just msbuild
and default
(or maybe call it console
?) for now.
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.
Renamed Startparameter and used enum as suggested.
Src/CSharpier.Cli/FileIssueLogger.cs
Outdated
{ | ||
public void WriteError(string value, Exception? exception = null) | ||
{ | ||
logger.LogError(exception, $"{this.GetPath()} - {value}"); | ||
if (isMsBuildFormat) | ||
logger.LogError("{Path}: error: {Message}", this.GetPath(), value); |
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.
I think the WriteWarning
message should also use msbuild format. Although I don't recall what is considered a warning by CSharpier, and we turn on warningsAsErrors
at work. Maybe that would cause issues.
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.
Also made WriteWarning() obey the --log-format parameter.
Also added a TODO comment for adding a test as soon as it is clear in which situations CSharpier will output warnings.
private const bool IsMsBuildFormat = true; | ||
private const bool IsNotMsBuildFormat = 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.
These don't appear to be used and should be removed.
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.
Removed leftovers :)
--msbuild-format
--out-format <console|msBuild>
parameter to format error message according to standard error/warning format of MSBuildEnables to double click on "was not formatted" message in error list of Visual Studio to jump to the file affected.