-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Include CodeLens on remaining types and members #69608
Conversation
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span)); | ||
} | ||
|
||
public override void VisitDestructorDeclaration(DestructorDeclarationSyntax node) |
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.
note: won't ever have results. so this may be likely to annoy people as it will always have "0 references" showing for it. Would prefer to not do this.
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 that's fine, for a few reasons:
- Behavior matches current Visual Studio behavior
- Destructors are rare and generally discouraged, so most users won't ever see the difference
- For the remaining users, it's less likely someone will ask why it shows 0 than for someone to ask why it's missing
public override void VisitDelegateDeclaration(DelegateDeclarationSyntax node) | ||
{ | ||
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span)); | ||
} |
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.
definitely approve of this.
{ | ||
_memberBuilder.Add(new CodeLensMember(variable, variable.Identifier.Span)); | ||
} | ||
} |
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.
- what's teh consequence of this in VS for multi-declarator members?
- does thsi light up code-lens for fields for all users in VS. if so, i think this should be gated behind a user option peopel can opt into.
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.
what's teh consequence of this in VS [Code] for multi-declarator members?
I'm not sure how to test the UI in VS Code, but I would expect it to be able to handle this since today it's already possible to declare multiple properties on the same line of code.
does thsi light up code-lens for fields for all users in VS
This has no impact on Visual Studio. This is a VS Code feature.
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 not sure how to test the UI in VS Code
@dibarbet can you guide sam on this?
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 not sure how to test the UI in VS Code, but I would expect it to be able to handle this since today it's already possible to declare multiple properties on the same line of code.
If you mean seeing the actual code lens items inside vscode, it should be relatively straightforward. Basically:
- Make sure the C# extension is installed and you can open a C# project
- Point the C# extension to your locally built server as described here - https://github.com/dotnet/vscode-csharp/blob/main/CONTRIBUTING.md#using-a-locally-developed-roslyn-server
- Reload the window
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.
Support the following items in the LSP implementation of CodeLens (used by VS Code, but is not used by Visual Studio):
Fixes #69583