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 client-side support for macro expansions #621

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

fwcd
Copy link
Contributor

@fwcd fwcd commented Oct 11, 2023

This implements basic client-side support for the sourcekit-lsp/macroExpansion request introduced in

The expansion is presented using an in-memory text document in a peeked editor:

image

To do

  • Declare the request (sourcekit-lsp/macroExpansion)
  • Add a VSCode command for performing the request (swift.expandMacro)
  • Find a better presentation style than dumping the expansion into an info message popup

@adam-fowler
Copy link
Member

The third todo in your list is going to be the stumbling block. I've looked into finding a better way to present macros a while back and didn't find an easy solution.

You mention rust-analyser in the related sourcekit-lsp PR. Do you know what they do to present macros?

@fwcd
Copy link
Contributor Author

fwcd commented Oct 11, 2023

They spawn another (regular) editor containing the expansion:

image

@adam-fowler
Copy link
Member

That's not really a great solution

@fwcd
Copy link
Contributor Author

fwcd commented Oct 11, 2023

I agree, haven't dug in too deep what APIs VSCode exposes in this regard, but I think it would be really cool if we could use an "inline editor" for this, which incidentally would also approximate Xcode's macro expansion UI:

image

I think GitLens does or used to do something similar. We might still have to register a virtual document provider, but that should probably be doable.

Edit: This presentation style seems to be called "Peeked editors" and there's an open upstream issue on exposing an API for creating custom peek widgets:

Since we only need to display, perhaps there is a still a way to present such an editor with the existing APIs. Another proposed API that might be of interest (which would require us to implement a custom view though):

@fwcd fwcd marked this pull request as ready for review October 13, 2023 13:38
@fwcd
Copy link
Contributor Author

fwcd commented Oct 13, 2023

The peeked editor approach seems to work well and looks nice too (check the PR description for a screenshot).

@DougGregor
Copy link

The peeked editor approach seems to work well and looks nice too (check the PR description for a screenshot).

That looks great, thank you!

Copy link
Member

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

It looks very nice. I didn't know about "editor.action.peekLocations"
In SourceKit-LSP you could add a CodeAction for macros to call the command you've setup here.

// old expansions will be closed correctly, but perhaps there's
// a more elegant solution to this?
const scheme = "swift-macro-expansion";
const provider = vscode.workspace.registerTextDocumentContentProvider(scheme, {
Copy link
Member

Choose a reason for hiding this comment

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

We will need to manage the text document content providers slightly better. Currently you just keep creating them, without cleaning up the unused ones. Adding them to the workspaceContext subscriptions means they get removed when the workspace is closed, but not before then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants