Parse #Playground macro expansions and include them in CodeLens request

[award999]

Support swift.play in textDocument/codelens request

• New swift.play CodeLens support that is an experimental feature while swift play is still experimental
• Add #Playground macro visitor to parse the macro expansions
• File must import Playgrounds to record the macro expansion
• The swift-play binary must exist in the toolchain to
• TextDocumentPlayground will record the id and optionally label to match detail you get from

$ swift play --list
Building for debugging...
Found 1 Playground
* Fibonacci/Fibonacci.swift:23 "Fibonacci"

• Add LSP extension documentation for designing pending workspace/playground request
• Add new parsing test cases
• Update CMake files

Issue: #2339 #2343

[ahoppen]

I haven’t looked at this in full detail yet but is there a reason why this needs to be an LSP extension and can’t be code lenses returned from the textDocument/codeLens request?

[award999]

@ahoppen that's the next step, here is commit award999@061075f was just waiting for this piece first before staging that PR

[award999]

is there a reason why this needs to be an LSP extension and can’t be code lenses returned from the textDocument/codeLens request?

intent is to be listing the Playgrounds in the project panel. We'll do a workspace/playgrounds request on startup for initial list, and on document changes, fetch the list of textDocument/playgrounds to update the list, without heavy weight operation of fetching whole list and updating the whole tree

[bnbarham]

is there a reason why this needs to be an LSP extension and can’t be code lenses returned from the textDocument/codeLens request?
intent is to be listing the Playgrounds in the project panel. We'll do a workspace/playgrounds request on startup for initial list, and on document changes, fetch the list of textDocument/playgrounds to update the list, without heavy weight operation of fetching whole list and updating the whole tree

I suppose it is a reasonable point that textDocument/playgrounds is unlikely to ever need a hierarchy (which is presumably the main reason textDocument/tests exists). So I could see arguments for either and regardless codeLens would have custom data for this case. I don't particularly mind either way, how strongly do you feel about having in textDocument/codeLens @ahoppen?

We'll do a workspace/playgrounds request on startup for initial list, and on document changes, fetch the list of textDocument/playgrounds to update the list

Worth noting that I was looking at test discovery with @rintaro recently and I believe we're not handling external changes today (I'm assuming test discovery following a similar process).

[ahoppen]

I know that we talked about extending this to a workspace/playgrounds request and that the textDocument/playgrounds request thus makes sense. I would like to see and discuss the structure of that declaration in this PR because I think that the two requests can heavily influence each other and decide if it would be a better fit to use textDocument/codeLens to get the playgrounds in a single document.

[award999]

@ahoppen sorry if some redundant comments were spamming you, GH is acting really weird for me, not showing some conversation comments as posted. IDK what's up

[rintaro]

Could you update CMakeLists.txt for the new files?

[award999]

Could you update CMakeLists.txt for the new files?

@rintaro done

[ahoppen]

A couple more small comments inline.

On a higher level, I have thought about using the textDocument/codeLens request to get the playgrounds within a single file instead of introducing textDocument/playgrounds and I am more certain that that’s we should design it because even if other editors don’t want to implement the workspace-wide playground discovery, it will allow them to pretty easily support playgrounds within a single file. It also nicely aligns with the swift.run and swift.debug commands that we already offer and having one fewer request is also nice. My proposed design would be that the client announces support for playgrounds by including swift.play in clientCapabilities.textDocument.codeLens.supportedCommands. The code lens then returns a command with a title that include the playground’s name and the playground’s ID in arguments. Or am I missing something that would make the implementation harder on the VS Code side with my proposed design?

[award999]

@ahoppen I did make the document playground response type just a range instead of whole location like you've asked for. I also updated the LSP Extensions doc to add the workspace/playgrounds request to make it easier to design what's next at the same time: https://github.com/swiftlang/sourcekit-lsp/pull/2340/files#diff-005ce06479af2f7c1f5ad69b549cb89b9c26b16b23417f54ec96a81b05c883edR745

My vote still is for a separate textDocument/playgrounds request. There are editors that don't support CodeLens, and some who can display the text but not the command execution. So I think it is not very intuitive that you need to know to issue the CodeLens request indicating that you support swift.play as a command and filter out the swift.run and swift.debug commands. Both requests are going to use the same PlaygroundFinder, so if you look at the CodeLens request that is the next PR I'll open: award999@b10cfec#diff-f8a9947dfe74d3bd634971372a2af1b1dc338e18ece6ab591e1f6d2ebed3c73aR74, there is not a lot of extra code to support both these requests and the documentation is a lot simpler to explain, i.e. use a combination of workspace/playgrounds and textDocument/playgrounds...

To answer your question about VSCode, no this does not cause an issue in VS Code but it is more code to make this happen, we'll need a CodeLens request middleware, filter out the CodeLens we care about, and then pass on those playground items for merging.

[award999]

After further discussion me and @ahoppen have agree we will not include a textDocument/playgrounds request right now, and VSCode can determine updates through the CodeLens request. Will wait and see if other editors have a compelling case where they want to get list of playgrounds but don't support CodeLens

[ahoppen]

Overall, this looks good to me, just a few nitpicky comments. Also, all the LSP type definitions are now in swift-tools-protocols, so you’ll need to

• Add the new types there
• Ask @owenv to tag a new release of swift-tools-protocols
• Update the version of swift-tools-protocols that’s used here and here
• Then you can test this PR again

I hope I didn’t forget any step and sorry for the extra complication with this new process…