Skip to content

Implement document symbol request #98

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

Merged

Conversation

Trzyipolkostkicukru
Copy link
Contributor

@Trzyipolkostkicukru Trzyipolkostkicukru commented Apr 14, 2019

I implemented document symbol request, which enables "document outline" in visual studio code.

I have no experience with contributing features to open source, I tried to match the style of the rest of the project. Are there any things I have to change/add?

Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR! The code changes are looking pretty good to a first look, I just left a few inline comments. I haven't dug into the details of the specification for this request, so I'll probably do another round of detailed review when I have a chance to look at that.

One important thing that's missing is tests. The FoldingRangeTests are a good model to look at for how to setup the test server, provide document contents, etc.

@benlangmuir
Copy link
Contributor

@swift-ci please test

Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

Hey, sorry for the slow response. Please add the constructor case, and then update the linux tests by running:

swift test --generate-linuxmain

Otherwise I think this is ready to go. Thanks!

@benlangmuir
Copy link
Contributor

@swift-ci please test

@benlangmuir benlangmuir merged commit 36e5673 into swiftlang:master May 30, 2019
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.

3 participants