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

Inline Diagnostics: Fix conflicting diagnostics and general code cleanup #60468

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

akhera99
Copy link
Member

No description provided.

@@ -284,7 +284,7 @@ protected bool ShouldDrawTag(SnapshotSpan snapshotSpan, IMappingTagSpan<T> mappi
}

var mappedPoint = TextView.BufferGraph.MapUpToSnapshot(
point.Value, PointTrackingMode.Negative, PositionAffinity.Predecessor, TextView.VisualSnapshot);
point.Value, PointTrackingMode.Negative, PositionAffinity.Predecessor, TextView.TextSnapshot);
Copy link
Member

Choose a reason for hiding this comment

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

Because I am curious, what is the difference between these?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not quite sure what the difference is to be honest, but I did MapUpToSnapshot earlier in the file as well and I used the TextSnapshot. This was just to keep consistency because I don't think I used the VisualSnapshot for a particular reason.

Copy link
Member

Choose a reason for hiding this comment

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

can you make sure that if you scroll these off screen that they disappear (debugging through to amek sure that happens would be good too).

var graphicsResult = tag.GetGraphics(TextView, geometry, GetFormat(classificationType));

// Pass in null! because the geometry is unused for drawing anything for Inline Diagnostics
var graphicsResult = tag.GetGraphics(TextView, null!, GetFormat(classificationType));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var graphicsResult = tag.GetGraphics(TextView, null!, GetFormat(classificationType));
var graphicsResult = tag.GetGraphics(TextView, unused: null!, GetFormat(classificationType));

Copy link
Member

Choose a reason for hiding this comment

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

👍

@akhera99 akhera99 enabled auto-merge March 30, 2022 16:33
@akhera99 akhera99 merged commit 91bd6e5 into dotnet:main Mar 30, 2022
@ghost ghost added this to the Next milestone Mar 30, 2022
@dibarbet dibarbet modified the milestones: Next, 17.3.P1 Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants