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

Fix the duplicate glyphs in Inheritance margin #73996

Merged
merged 10 commits into from
Jun 14, 2024

Conversation

Cosifne
Copy link
Member

@Cosifne Cosifne commented Jun 14, 2024

Fix: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1988154/
Currently, you might find there are duplicate glyphs in inheritance margin like:
image
(There are three controls created in the canvas, all of them are same)

Reason:
Our tagger infra has switched to a three-tagger stucture.
Tagger 1 -> Tag the [visible region + 10 line, visible region - 10 line]
Tagger 2 -> Tag the [visible region + 10 line, visible region + 100 line]
Tagger 3 -> Tag the [visible region - 100 line, visible region - 10 line]

This change causes problem here

So computing the symbol to tag, inheritance margin service would call root.DescendantNodes(spanToSearch) to find all the nodes intersect with spanToSearch.
So suppose you have a class, and the visible region is around class A

//1k lines comments
class A
{
// 1k lines body
}

Tagger 1 would tag class A. (Because the spanToSearch is the visible span)
Tagger 2 would also tag class A, because the top lines span also intersects with class A
Tagger 3 would also tag class A, because the empty body span also intersects with class A

The solution is to only create the tag when spanToSearch contains the member's identifier.

@Cosifne Cosifne requested a review from a team as a code owner June 14, 2024 00:31
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 14, 2024
@@ -523,9 +524,9 @@ public Task TestCSharpSpecialMember(string memberDeclaration, TestHost testHost)
var markup = $@"
public abstract class {{|target1:Bar1|}}
{{}}
public class Bar : Bar1
public class {{|{SearchAreaTag}:Bar : Bar1
Copy link
Member Author

Choose a reason for hiding this comment

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

This test is modified a little bit.
The prev test is asserting that no inheritanceMargin tag should be created for special C# methods. (e.g. constructor/deconsctructor/operator)

So the assertion is just assert one tag is created for the type (class Bar1 here)

It only chooses to search the memberDeclaration region so only tags for Bar1 would be created.

However, after the change, we would only return the tag within the search span. So expand the SearchAreaTag area here to include class Bar1
No real change here.

@DoctorKrolic
Copy link
Contributor

Fixes #73615?

@Cosifne
Copy link
Member Author

Cosifne commented Jun 14, 2024

Fixes #73615?

Yes it should fix the problem

@Cosifne Cosifne merged commit e102ec2 into dotnet:main Jun 14, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jun 14, 2024
@jjonescz jjonescz modified the milestones: Next, 17.11 P3 Jun 24, 2024
@Cosifne Cosifne deleted the dev/shech/dedupTags branch July 3, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants