KirillOsenkov / MSBuildStructuredLog

A logger for MSBuild that records a structured representation of executed targets, tasks, property and item values.
MIT License
1.41k stars 188 forks source link

Code and File metadata not visible or searchable for non-critical messages #708

Closed rainersigwald closed 8 months ago

rainersigwald commented 8 months ago

If a regular Messsage event has Code/File/Line metadata, it's not shown in the viewer, and it's also not searchable. This makes the experience for messages with codes a bit unpleasant. The built-in text loggers do render these fields even for non-Critical messages.

Repro

<Project>
 <UsingTask
    TaskName="LogWithCodeAndFile"
    TaskFactory="RoslynCodeTaskFactory"
    AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll" >
    <ParameterGroup />
    <Task>
      <Code Type="Fragment" Language="cs">
        Log.LogMessage(null, "ABC1234", null, @"$(MSBuildThisFileFullPath)", 2, 0, 0, 0, MessageImportance.High, "Message text");
      </Code>
    </Task>
  </UsingTask>

 <Target Name="Message">
  <LogWithCodeAndFile />
 </Target>
</Project>
❯ msbuild .\MessageWithCodeAndLocation.proj -tl:false -bl -v:m
MSBuild version 17.9.0-preview-23518-06+c6f4fc134 for .NET Framework

S:\play\MessageWithCodeAndLocation.proj(2): message ABC1234: Message text

MessageWithCodeAndLocation.zip

image

jaredpar commented 8 months ago

@KirillOsenkov is there a reason these items are suppressed in the viewer? This is a bug I'm more than happy to take a stab at fixing but wanted to make sure I had context for reasons why this may have been done. That way I don't accidentally undo desired behavior.

KirillOsenkov commented 8 months ago

No reason I can think of, feel free to send a PR.

Be mindful of performance, there are millions of messages. Not sure if this is to be done at the WPF template level or just string concat for the text. If only a small portion of messages have an error code, I think just prepend text is easier because it will take care of searchability too. Curious what we do for warnings and errors.

If we need to make more fields searchable, NodeQueryMatcher is the place where you can register more fields to be searched, but if you just prepend the code to the text it might not be needed.

For file and line, the behavior should be that you press Space on the message and it opens the source file in the text editor to the right. Is this currently working?

rainersigwald commented 8 months ago

If only a small portion of messages have an error code

I would expect this to be a very small portion.

For file and line, the behavior should be that you press Space on the message and it opens the source file in the text editor to the right. Is this currently working?

No, no effect that I can see from space bar on that message.

KirillOsenkov commented 8 months ago

OK here you go:

image

Space now works too to go to the location.

I found only one real world case in a couple of binlogs I tried, where this is used, namely NU1702: NU1702: ProjectReference foo.csproj' was resolved using '.NETFramework,Version=v4.7.2' instead of the project target framework '.NETCoreApp,Version=v6.0'. This project may not be fully compatible with your project.

rainersigwald commented 8 months ago

The one that got me to file this was NETSDK1187. Definitely not common, but the fix looks great, thanks!

jaredpar commented 8 months ago

Thank you!