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

Implement basic textDocument/documentSymbol #99

Merged
merged 6 commits into from May 1, 2023

Conversation

kubukoz
Copy link
Contributor

@kubukoz kubukoz commented Apr 22, 2023

Issue #, if available:

Closes #87.

Description of changes:

Implements a basic documentSymbolProvider.

Some limitations of this implementation that may be expanded further in future changes:

  • flat structure: things like operations could have child symbols, but currently everything is on the top level
  • some SymbolKind translations may be made more precise with e.g. member target lookup

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Demo:

image

image

image

@kubukoz kubukoz requested a review from a team as a code owner April 22, 2023 01:23
@srchase
Copy link
Contributor

srchase commented Apr 26, 2023

Could you fix up the checkstyle issues (star import, etc) so we can confirm the rest of CI is successful?

@kubukoz
Copy link
Contributor Author

kubukoz commented Apr 26, 2023

done! I can't imagine how you folks can work like this, without an automated rewrite that fixes these issues ;)

@kubukoz
Copy link
Contributor Author

kubukoz commented Apr 26, 2023

let's see if the file-based way works on Windows...

@kubukoz
Copy link
Contributor Author

kubukoz commented Apr 26, 2023

Apparently not. Sadly I have no access to a Windows machine so won't be able to play with it. @srchase any hints for resolving this?

@srchase srchase mentioned this pull request Apr 26, 2023
@srchase
Copy link
Contributor

srchase commented Apr 27, 2023

Apparently not. Sadly I have no access to a Windows machine so won't be able to play with it. @srchase any hints for resolving this?

This change fixed the Windows path issue: 0555cfa

The issue is that on Windows, documentUri looked like this: file:/C:/..., whereas loc.getUri() is file:C:\..., so the fix normalizes the slashes and then only compares the last segment when split on the colon.

@kubukoz
Copy link
Contributor Author

kubukoz commented Apr 27, 2023

awesome! let me see if it still works as expected on my machine and I'll pull that in.

@kubukoz
Copy link
Contributor Author

kubukoz commented Apr 27, 2023

looks alright :)

@srchase srchase merged commit d20c5c2 into smithy-lang:main May 1, 2023
18 checks passed
@kubukoz kubukoz deleted the document-symbol-provider branch May 1, 2023 22:20
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.

[feature-request] become a documentSymbolProvider
2 participants