Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Move to DocumentSymbol #1795

Merged

Conversation

JacksonKearl
Copy link

@JacksonKearl JacksonKearl commented Jul 17, 2018

this closes #1772

@JacksonKearl JacksonKearl changed the title Begin move to DocumentSymbol Move to DocumentSymbol Jul 18, 2018
@JacksonKearl JacksonKearl self-assigned this Jul 18, 2018
@JacksonKearl JacksonKearl added this to the 0.6.82 milestone Aug 21, 2018
@vladbarosan vladbarosan self-assigned this Feb 21, 2019
@vladbarosan
Copy link
Contributor

@JacksonKearl I hope you don't mind I updated the PR to the latest master and refactored it a little

@vladbarosan
Copy link
Contributor

@JacksonKearl , @ramya-rao-a , @jhendrixMSFT if you can have a look since its a change that touches multiple parts. Still testing all the functionality touched. will update when im done

@vladbarosan vladbarosan force-pushed the trransition-to-documentsymbols branch 5 times, most recently from 1ebdc18 to 6e3393c Compare February 22, 2019 07:32
src/goGenerateTests.ts Outdated Show resolved Hide resolved
src/goOutline.ts Show resolved Hide resolved
label,
decl.type,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the "details" that is shown next to the symbol in the outline feature?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes ( so in this case would look like the name and next to it the type ( e.g. package, function etc)

Copy link
Contributor

Choose a reason for hiding this comment

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

also looking in go-outline, everything seems fine, but we stop only at the function level. do we own https://github.com/lukehoban/go-outline ? seems we could get more from the AST

Copy link
Contributor

Choose a reason for hiding this comment

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

We own the fork of it https://github.com/ramya-rao-a/go-outline
Feel free to look into that for improvements

Copy link
Contributor

Choose a reason for hiding this comment

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

well it seems we might be able to get more info from the AST, of things inside the functions, @jhendrixMSFT has more experience with the Go AST, he can confirm.

Copy link
Member

Choose a reason for hiding this comment

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

Absolutely. I've opened #2370 to track enhancing the outline view.

src/goGenerateTests.ts Outdated Show resolved Hide resolved
@vladbarosan vladbarosan force-pushed the trransition-to-documentsymbols branch from 33d9d34 to e9ebecd Compare March 5, 2019 23:07
@vladbarosan vladbarosan force-pushed the trransition-to-documentsymbols branch from e9ebecd to aba4a83 Compare March 5, 2019 23:55
@vladbarosan
Copy link
Contributor

@ramya-rao-a @jhendrixMSFT @JacksonKearl everything touched seemed fine on testing, so if you want to double check, otherwise from my side I think its good to go.

@ramya-rao-a ramya-rao-a merged commit a838a70 into microsoft:master Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adopt DocumentSymbol API
4 participants