-
Notifications
You must be signed in to change notification settings - Fork 70
lsp: textDocument/documentSymbol support #195
base: master
Are you sure you want to change the base?
Conversation
vscode does not understand flow annotation.
86b6174
to
90c4c58
Compare
@asiandrummer could you ptal at this and #196? |
Yeah sorry - I'll review this now |
Field: SymbolKind.Field, | ||
OperationDefinition: SymbolKind.Class, | ||
FragmentDefinition: SymbolKind.Class, | ||
FragmentSpread: SymbolKind.Struct, |
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.
Should we look to add more symbols in the future, e.g. ObjectTypeExtension
and such?
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.
Sounds reasonable to me - I haven't worked very much with the SDL, so I only implemented what the outline code already does here: https://github.com/stephen/graphql-language-service/blob/90c4c5899f3f14216794636749210386fa749427/packages/interface/src/getOutline.js#L21-L30
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.
Thanks for implementing this - I'll cut a release with a fix I'm planning to add soon.
This is awesome, merge pls? 🙏 |
@asiandrummer how can i convince y'all to take a look at this again? Happy to rebase and sort out merge conflicts if that's all it needs. |
I'm happy to take a look (next week at the soonest) |
Looks good to me. @stephen if you could rebase etc then I think it should be merged. |
Any update? This would be really nice to have. @stephen |
well well well, time to recreate this for the new monorepo at https://github.com/graphql/graphiql! some of this is gonna have to be re-done in TS but the server is still made with flow. can't wait to have this in monaco -> new graphiql too :D @stephen @lostplan @nathanchapman what's the move? if i dont hear anything in ~1 week, I'll go ahead and take care of it, with full dev/review attribution of course! trying to get this merged in the next month or so, my bad for accidentally leaving this hanging after the migration, so important! and it was ready! le sigh. |
This PR adds support for
textDocument/documentSymbol
to the lsp-compatible server.Here's a screenshot in vscode: