-
Notifications
You must be signed in to change notification settings - Fork 302
Allow configuring of SourceKit-LSP’s options using configuration files #1524
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
Conversation
@swift-ci Please test |
Documentation/Configuration File.md
Outdated
- `~/.sourcekit-lsp/.sourcekit-lsp` (ie. a `.sourcekit-lsp` file in the `~/.sourcekit-lsp` directory) | ||
- A `.sourcekit-lsp` file in a workspace’s root |
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.
What about .sourcekit-lsp/config
and require the directory in the workspace root as well? Seems plausible we may add more later. And IMO ideally we'd check Library/org.swift.sourcekit-lsp/config
for macOS -> $XDG_CONFIG_HOME/sourcekit-lsp/config
everywhere (though not sure what the standard is for Windows) -> .sourcekit-lsp/config
.
I do wonder if we need to keep the server options still as well - consider experimental features that may only be supported by specific editors + a user that jumps between editors. Though arguably that case might not work even if you could configure them differently. This also makes it more difficult for editors to add support for specific options.
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.
Good ideas. I modified the search locations slightly to
~/.sourcekit-lsp/config.json
- On macOS:
~/Library/Application Support/org.swift.sourcekit-lsp/config.json
from the variousLibrary
folders on the system - If the
XDG_CONFIG_HOME
environment variable is set:$XDG_CONFIG_HOME/org.swift.sourcekit-lsp/config.json
- A
.sourcekit-lsp/config.json
file in a workspace’s root
.sourcekit-lsp
configuration filesThis extension was added for VS Code but never used. Let’s remove it in favor of workspace-specific configuration files.
@swift-ci Please test |
@swift-ci Please test Windows |
@swift-ci Please test |
@swift-ci Please test Windows |
public init( | ||
textDocument: TextDocumentIdentifier, | ||
position: Position, | ||
context: CompletionContext? = nil, | ||
sourcekitlspOptions: SKCompletionOptions? = nil |
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.
There's a doc comment for this parameter above, I guess it should be removed?
…onfiguration files The idea here is to unify the different ways in which we can currently set options on SourceKit-LSP in a scalable way: Environment variables, command line arguments to `sourcekit-lsp` and initialization options. The idea is that a user can define a `~/.sourcekit-lsp/.sourcekit-lsp` file (we store logs in `~/.sourcekit-lsp/logs` on non-Darwin platforms), which will be used as the default configuration for all SourceKit-LSP instances. They can also place a `.sourcekit-lsp` file in the root of a workspace to configure SourceKit-LSP for that project specifically, eg. setting arguments that need to be passed to `swift build` for that project and which thus also need to be set on SourceKit-LSP. For compatibility reasons, I’m mapping the existing command line options into the new options structure for now. I hope to delete the command line arguments in the future and solely rely on `.sourcekit-lsp` configuration files. Environment variable will be migrated to `.sourcekit-lsp` in a follow-up commit.
@swift-ci Please test |
@swift-ci Please test Windows |
1 similar comment
@swift-ci Please test Windows |
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.
IMO the XDG_CONFIG_HOME should be just sourcekit-lsp rather than reverse-domain. I'd also specify the configuration in LSP and then allow taking it in initializationOptions. But feel free to merge and fix in a follow up 👍
@swift-ci Please test Windows |
The idea here is to unify the different ways in which we can currently set options on SourceKit-LSP in a scalable way: Environment variables, command line arguments to
sourcekit-lsp
and initialization options.The idea is that a user can define a
~/.sourcekit-lsp/.sourcekit-lsp
file (we store logs in~/.sourcekit-lsp/logs
on non-Darwin platforms), which will be used as the default configuration for all SourceKit-LSP instances. They can also place a.sourcekit-lsp
file in the root of a workspace to configure SourceKit-LSP for that project specifically, eg. setting arguments that need to be passed toswift build
for that project and which thus also need to be set on SourceKit-LSP.For compatibility reasons, I’m mapping the existing command line options into the new options structure for now. I hope to delete the command line arguments in the future and solely rely on
.sourcekit-lsp
configuration files.Environment variable will be migrated to
.sourcekit-lsp
in a follow-up commit.