Skip to content

Allow macro expansions to be viewed through GetReferenceDocumentRequest instead of storing in temporary files #1567

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

Merged

Conversation

lokesh-tr
Copy link
Contributor

@lokesh-tr lokesh-tr commented Jul 17, 2024

This PR eliminates the storage of temporary macro expansion files for PeekDocumentsRequest by introducing a new LSP Extension GetReferenceDocumentRequest.

The implementation is made in such a way that the URI passed for macro expansions through PeekDocumentsRequest is customised with a new scheme, and expects the client to resolve the contents of this new URI by making a GetReferenceDocumentRequest.

Custom Scheme for Macro Expansions:
`sourcekit-lsp://swift-macro-expansion/LaCb-LcCd.swift?primaryFilePath=&fromLine=&fromColumn=&toLine=&toColumn=&bufferName=`

References:

Previous PR which implemented PeekDocumentsRequest: #1479
Accompanying PR in vscode-swift repository: swiftlang/vscode-swift#971


Expansion of Swift Macros in Visual Studio Code - Google Summer Of Code 2024
@lokesh-tr @ahoppen @adam-fowler

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Wow, this was easier than I expected.

@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch from c9e559b to 58a1d7d Compare July 18, 2024 08:10
@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch from 58a1d7d to f49c429 Compare July 25, 2024 14:41
@lokesh-tr lokesh-tr requested a review from ahoppen July 25, 2024 14:55
@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch from f49c429 to df712a5 Compare July 25, 2024 15:38
@lokesh-tr
Copy link
Contributor Author

@ahoppen I have updated the PR and fixed the rebase issue with package specifiers. Ready for full review now. 👍🏻

@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch 2 times, most recently from 7f495e1 to a8f496d Compare July 26, 2024 10:13
@lokesh-tr
Copy link
Contributor Author

@ahoppen ready for review

@lokesh-tr lokesh-tr requested a review from ahoppen July 26, 2024 10:23
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Looks much better with the split for the URL data in my opinion.

@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch from a8f496d to c9f9d23 Compare July 29, 2024 13:48
@lokesh-tr
Copy link
Contributor Author

@ahoppen Ready for review.

@lokesh-tr lokesh-tr requested a review from ahoppen July 29, 2024 13:52
@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch 2 times, most recently from 87026f7 to b6d9a02 Compare July 31, 2024 12:25
@lokesh-tr lokesh-tr force-pushed the macro-expansions-get-reference-document branch from b6d9a02 to 0522e1a Compare July 31, 2024 13:33
@lokesh-tr
Copy link
Contributor Author

@ahoppen Fixed everything as you suggested 🙂

@lokesh-tr lokesh-tr requested a review from ahoppen July 31, 2024 13:36
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Very nice

@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2024

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge July 31, 2024 13:55
@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 5c2055d into swiftlang:main Jul 31, 2024
3 checks passed
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.

2 participants