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

Added attribute span property for fileMemberElement #1075

Merged
merged 2 commits into from Jan 11, 2018

Conversation

akshita31
Copy link
Contributor

Issue : dotnet/vscode-csharp#429

Added properties to contain the attribute span in a file member element, which will be used by the editor to display the "# of references" before the attributes.

@akshita31
Copy link
Contributor Author

Omnisharp-vscode side : dotnet/vscode-csharp#1938

Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

makes sense to me

Copy link

@rchande rchande left a comment

Choose a reason for hiding this comment

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

No tests?

@@ -9,6 +9,10 @@ public class FileMemberElement : IComparable<FileMemberElement>

public QuickFix Location { get; set; }

public int AttributeSpanStart { get; set; }
Copy link

Choose a reason for hiding this comment

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

Can we make these some kind of range or span type?

Copy link
Contributor Author

@akshita31 akshita31 Jan 3, 2018

Choose a reason for hiding this comment

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

@rchande We could pass both AttributeSpanStart and AttributeSpanEnd in a Span type class. But in that case we will also have to define this class in protocol.ts in omnisharp-vscode. Should that be done ?

Copy link

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rchande Yes it does, but the range has the start and end as Point type. And I couldn't spot a function in omnisharp-roslyn that converts from a span to range

@david-driscoll david-driscoll merged commit 290c9d2 into OmniSharp:master Jan 11, 2018
@akshita31 akshita31 deleted the codelens_attr branch January 11, 2018 21:34
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.

None yet

4 participants