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

Add protocol conformance and super class details to DocumentSymbol for types #1097

Open
adam-fowler opened this issue Feb 28, 2024 · 7 comments

Comments

@adam-fowler
Copy link
Contributor

The LSP DocumentSymbol interface includes a details field. Currently SourceKit-LSP leaves this field empty.

For classes, struct and actors would it be possible to include the protocol conformances and super class in this field e.g. MyClass: SuperClass, MyStruct: Sendable, Equatable etc

@ahoppen
Copy link
Collaborator

ahoppen commented Feb 28, 2024

Tracked in Apple’s issue tracker as rdar://123752547

@ahoppen
Copy link
Collaborator

ahoppen commented Mar 1, 2024

It would definitely possible. Do you think it would be useful though? I would think that it produces more noise than value.

@adam-fowler
Copy link
Contributor Author

It would be useful for me to recognise XCTestCase classes. So I can add test items for new test classes as they are written (and not wait for a compilation). Currently the test update between builds can only add methods to existing test classes.

Also it would be interesting to see how providing them affects the file outline view.

@ahoppen
Copy link
Collaborator

ahoppen commented Mar 4, 2024

I was mostly thinking about the outline view where I think it would just add noise.

For the test explorer, I think we should be relying on workspace/tests and textDocument/tests requests instead of the document symbols request. I’m not entirely sure what you’re doing right now (looking forward to the PR that starts using workspace/tests and textDocument/tests) but if we use workspace/tests and textDocument/tests, the test discovery would be agnostic of XCTest and could also works with swift-testing.

@adam-fowler
Copy link
Contributor Author

My main concern with relying on workspace/tests and textDocument/tests is they require a build.
By the way the PR for workspace/tests has been merged into main. If you are able to build the extension you can see it working.

@ahoppen
Copy link
Collaborator

ahoppen commented Mar 6, 2024

Oh, I didn’t think about the fact that textDocument/tests uses the index but that you currently want it to be up-to-date even without rebuilding the file.

What I would prefer is that I change textDocument/tests to use the index if it is up-to-date and if it is not, do a syntactic scan in sourcekit-lsp. That way you wouldn’t need to do scraping in the extension. With that in mind: What are the information that you would need? We can structure the textDocument/tests request however we like. Could be something that’s more akin to textDocument/documentSymbol with nested symbols or a flat list like we have right now.

@adam-fowler
Copy link
Contributor Author

If I had a textDocument/tests request that used a syntactic scan then I could do a request on document save and I wouldn't need to parse the textDocument/documentSymbol response. A flat list would be fine.

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

No branches or pull requests

2 participants