- 
                Notifications
    You must be signed in to change notification settings 
- Fork 320
Expand trailing closures of code completion items #1072
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
Expand trailing closures of code completion items #1072
Conversation
| @swift-ci Please test | 
9e646d1    to
    408cd33      
    Compare
  
    | CI testing passed on macOS and Linux. This PR depends on two swift-syntax PRs (swiftlang/swift-syntax#2503 and swiftlang/swift-syntax#2506). Will re-trigger once they are merged. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main concern here is performance across many completions. Any idea how this performs on a relatively large file with thousands of completions?
408cd33    to
    23ae42b      
    Compare
  
    | Changed it to only parse the completion item itself instead of doing an incremental parse of the entire file. Measured performance and expanding placeholders in a single item takes ~0.2ms. So in the worst case if all 200 possible completion items have closure placeholders, it would could down completion by 40ms. Realistically, for example completion on  | 
| @swift-ci Please test | 
23ae42b    to
    c3a273b      
    Compare
  
    | @swift-ci Please test | 
| @swift-ci Please test Windows | 
Editors other than Xcode don’t have a notion of editor placeholders or their expansion, so we can’t produce results with editor placeholders and expect the user to expand them while completing the function arguments. Instead, expand all trailing closure placeholders when producing the code completion results. The generated expansion is currently not formatted with respect to the file’s indentaiton. Since we don’t want to launch `swift-format` for every completion item, this formatting will need to be done using `BasicFormat` which needs to infer the file’s indentation. Doing so will be non-trivial work on its own and will be done in a follow-up PR. rdar://121130170
c3a273b    to
    90c124c      
    Compare
  
    | @swift-ci Please test | 
| @swift-ci Please test Windows | 
sourcekit-lsp will be using this in swiftlang/sourcekit-lsp#1072 and Windows uses the swift-syntax modules from the compiler build for sourcekit-lsp.
| @swift-ci Please test Windows | 
Editors other than Xcode don’t have a notion of editor placeholders or their expansion, so we can’t produce results with editor placeholders and expect the user to expand them while completing the function arguments.
Instead, expand all trailing closure placeholders when producing the code completion results.
The generated expansion is currently not formatted with respect to the file’s indentaiton. Since we don’t want to launch
swift-formatfor every completion item, this formatting will need to be done usingBasicFormatwhich needs to infer the file’s indentation. Doing so will be non-trivial work on its own and will be done in a follow-up PR (rdar://123287930).rdar://121130170