Skip to content
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

Proper support for multiline messages #9699

Merged
merged 26 commits into from
Mar 21, 2024

Conversation

MichalPavlik
Copy link
Member

@MichalPavlik MichalPavlik commented Feb 1, 2024

Fixes #9666

Context

Adding proper support for multiline messages (with indentation).
Removed project name from message line.

Changes Made

Terminal logger uses different parsing and rendering.

Testing

Unit tests were updated + manual testing (Windows)

Notes

New output:

MSBuild version 17.10.0-dev-24101-01+36ab632fa for .NET Framework
  XUnitTestProject succeeded with warnings (0,2s)
    C:\TestProjects\XUnitTestProject\XUnitTestProject\XUnitTestProject.csproj(26,0): warning : 
      A
      Multi
      Line
      Warning.

@rainersigwald
Copy link
Member

Oh, just had a thought about moving the first line: @baronfel that would break the VS Code problem matcher, wouldn't it?

@baronfel
Copy link
Member

baronfel commented Feb 2, 2024

Oof, you're right. We'd need to check and update those

@MichalPavlik
Copy link
Member Author

I checked the matcher regex and it doesn't (for example) support all combinations we have for line/column info. For example case when lineNumber != 0 && columnNumber == 0 && endLineNumber != 0.

@MichalPavlik
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalPavlik
Copy link
Member Author

As discussed offline, we will not wait for VSCode update.

src/MSBuild/TerminalLogger/TerminalLogger.cs Outdated Show resolved Hide resolved
Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

Very excited for this to land, thank you!

@MichalPavlik MichalPavlik requested a review from a team as a code owner March 21, 2024 13:27
@ladipro
Copy link
Member

ladipro commented Mar 21, 2024

@MichalPavlik the diff is now showing 277 modified files. Something didn't go well with rebase/merge.

@ladipro ladipro self-requested a review March 21, 2024 13:38
@MichalPavlik
Copy link
Member Author

I had to rebase to register this feature. I'm not sure what happened, but these additional changes should reflect current main.

MichalPavlik and others added 13 commits March 21, 2024 15:25
…he logic for appending a string to a `StringBuilder` object has been modified. The change corrects a potential issue where `endColumnNumber` was being used instead of `columnNumber` when `endLineNumber` is 0.

Changes:
1. In the `TerminalLogger` class, the logic for appending a string to a `StringBuilder` object has been updated. Previously, if `endLineNumber` was 0, a string formatted with `lineNumber` and `endColumnNumber` would be appended. Now, a string formatted with `lineNumber` and `columnNumber` is appended instead. This change corrects a potential issue where `endColumnNumber` was being used instead of `columnNumber` when `endLineNumber` is 0.
Co-authored-by: Ladi Prosek <laprosek@microsoft.com>
MichalPavlik and others added 6 commits March 21, 2024 15:37
…he logic for appending a string to a `StringBuilder` object has been modified. The change corrects a potential issue where `endColumnNumber` was being used instead of `columnNumber` when `endLineNumber` is 0.

Changes:
1. In the `TerminalLogger` class, the logic for appending a string to a `StringBuilder` object has been updated. Previously, if `endLineNumber` was 0, a string formatted with `lineNumber` and `endColumnNumber` would be appended. Now, a string formatted with `lineNumber` and `columnNumber` is appended instead. This change corrects a potential issue where `endColumnNumber` was being used instead of `columnNumber` when `endLineNumber` is 0.
Co-authored-by: Ladi Prosek <laprosek@microsoft.com>
…_featureStatusMap` dictionary in the `Features` class. This feature, named `"TerminalLogger_MultiLineHandler"`, is set to `FeatureStatus.Available`, indicating that the TerminalLogger now has improved support for handling multi-line messages.

List of Changes:
1. A new feature `"TerminalLogger_MultiLineHandler"` was added to the `_featureStatusMap` dictionary in the `Features` class within the `Microsoft.Build.Framework` namespace. This feature is set to `FeatureStatus.Available`, suggesting enhanced support for multi-line messages in TerminalLogger.

Reference to Code Changes:
- Addition of `"TerminalLogger_MultiLineHandler"` feature to `_featureStatusMap` dictionary in `Features` class.
@MichalPavlik MichalPavlik force-pushed the dev/mipavlik/terminallogger-multiline-messages branch from 8db59d9 to 9b6e5ae Compare March 21, 2024 14:50
@MichalPavlik
Copy link
Member Author

I reconstructed commit stream on top of main, but it looks like I'm removing Jakub's changes for some reason....

@MichalPavlik MichalPavlik merged commit 6320d7b into main Mar 21, 2024
9 checks passed
@MichalPavlik MichalPavlik deleted the dev/mipavlik/terminallogger-multiline-messages branch March 21, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can TerminalLogger calm down on multi-line error/warning messages?
4 participants