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.

Sign up for GitHub

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

Jump to bottom

Update Editors/README.md #1184

Merged
merged 8 commits into from
May 15, 2024
Merged

Update Editors/README.md #1184

Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d826569
Update README.md
yaroslavyaroslav Apr 18, 2024
18b7c32
Review fixes
yaroslavyaroslav Apr 18, 2024
63fe6c1
Update README.md
yaroslavyaroslav Apr 20, 2024
4d06e65
Update README.md
yaroslavyaroslav Apr 21, 2024
87f9aa4
Update README.md
yaroslavyaroslav Apr 21, 2024
19d1893
Update README.md
yaroslavyaroslav Apr 21, 2024
8e7df19
Update README.md
yaroslavyaroslav Apr 23, 2024
491103d
Update README.md
yaroslavyaroslav Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 8 additions & 46 deletions Editors/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,56 +22,18 @@ The [Swift for Visual Studio Code extension](https://marketplace.visualstudio.co

## Sublime Text

Before using SourceKit-LSP with Sublime Text, you will need to install the LSP package from Package Control. To configure SourceKit-LSP, open the LSP package's settings. The following snippet should be enough to get started with Swift.
Before using SourceKit-LSP with Sublime Text, you will need to install the LSP and LSP-SourceKit packages from Package Control. And you're ready to go if you have `xcrun` in the `$PATH`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change this to sourcekit-lsp instead of xcrun because we don’t actually invoke xcrun.

Suggested change
Before using SourceKit-LSP with Sublime Text, you will need to install the LSP and LSP-SourceKit packages from Package Control. And you're ready to go if you have `xcrun` in the `$PATH`.
Before using SourceKit-LSP with Sublime Text, you will need to install the LSP and LSP-SourceKit packages from Package Control. And you're ready to go if you have `sourcekit-lsp` in the `$PATH`. This is the case if you have Xcode installed on macOS or a [Swift Toolchain](https://www.swift.org/install/) installed on Linux.


You will need the path to the `sourcekit-lsp` executable for the "command" section.
To configure LSP-SourceKit additionally, open the LSP-SourceKit package's settings by typing in command palette `Preferences: LSP-SourceKit Settings`. The following snippet should be enough to get started with Swift and Objective-C/C++ and the custom path to `sourcekit-lsp` executable.

To make your journey even more pleasant it's recomended to install [Swift-Next](https://github.com/Swift-Next/Swift-Next) package that provides advanced swift syntax highlighting support.
Copy link

@rchl rchl Apr 20, 2024 •
edited

Choose a reason for hiding this comment

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

The LSP-SourceKit package recommends the Decent Swift Syntax. Why the discrepancy? Should the package's readme be updated?

But also it's not so much about being "more pleasent". A syntax is required for LSP to work. So I'm not sure what "more pleasent" refers to? Not having the syntax or to having some other syntax? In the latter case, I'm not sure if that's clear because nothing here mentions to install the other syntax either. If it's really better then you could update the LSP-SourceKit readme and then omit the whole paragraph here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe we have this talk before, when Swift-Next got included in Package Control initially. So after all folks decided to not replace any existing ST swift syntax package with this one, but to add tests and add it into the default packages list instead. In short the main reason was that this package is ST4 compatible only, which is not the case for the "default" Swift package provides ST3 and ST2 support as well. So here we are. The only thing I can say to this: someday I'll complete the tests task, but the day is not today.

The same reasoning is true when we're speaking about the replacement of Decent Swift Syntax with Swift-Next in LSP-SourceKit package readme. If it's acceptable to suggest the ST4 only option as the default option for this package — i'm up for it, and would love to update its readme as well. But if you prefer to keep the one that provides better backward capability, so there's nothing to do with LSP-SourceKit's readme.

UPD: As you've mentioned in the comment below:

but ST3 has not been relevant for many years now. ST4 is what matters.

I believe that this is yes to the question above, thus I'm going to update LSP-SourceKit readme with replacement of a syntax suggestion as well soonish.

Copy link

@rchl rchl Apr 21, 2024 •
edited

Choose a reason for hiding this comment

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

Yes, ST3 is not relevant anymore.
And also LSP-SourceKit is ST4-only so if "Swift-Next" is better, it should just recommend that.

But what I've mainly tried to say is that from the perspective of someone who just reads intructions here, there is not even a mention of installing any syntax so this is why I thought the wording here is not ideal. Many users will likely not read LSP-SourceKit readme separately if they are installing it based on those instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I see your point, but personally I think that it's unlikely that a user would bother to install an lsp plugin for language while intentionally skipping its syntax highlighting part. So this is why I chose this form rather than more strict one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that we should make the suggestion for the syntax highlighting package consistent between the documentation of LSP-SourceKit and this REAMDE. I take it that there is consensus that Swift Next is superior to Decent Swift Syntax (especially because there is a plan to get Swift Next into the default packages) except for the part that Swift Next is not ST4-compatible. But if LSP-SourceKit isn’t either, I don’t think that’s a blocker. Could we

  • Change LSP-SourceKit to also refer to Swift Next?
  • I agree that we should rephrase this sentence to be a little less optional, on the lines of: Also install Swift Next to use Swift in ST.
  • I am not familiar with ST’s package infrastructure. But would it be possible to make LSP-SourceKit depend on Swift Next so users get the entire bundle when installing LSP-SourceKit?

Copy link

Choose a reason for hiding this comment

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

Change LSP-SourceKit to also refer to Swift Next?

It's updated now - sublimelsp/LSP-SourceKit#6

I am not familiar with ST’s package infrastructure. But would it be possible to make LSP-SourceKit depend on Swift Next so users get the entire bundle when installing LSP-SourceKit?

It's not technically possible right now for one package to depend on another package. There is a concept of dependencies but those are not packages and can't be installed on its own.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's updated now - sublimelsp/LSP-SourceKit#6

Thanks!

It's not technically possible right now for one package to depend on another package. There is a concept of dependencies but those are not packages and can't be installed on its own.

Ah, OK, I wasn’t aware of that. Let’s keep the suggestion in here then.


```json
{
"clients":
{
"SourceKit-LSP":
{
"enabled": true,
"command": [
"<sourcekit-lsp command>"
],
"env": {
// To override the toolchain, uncomment the following:
// "SOURCEKIT_TOOLCHAIN_PATH": "<path to toolchain>",
},
"languages": [
{
"scopes": ["source.swift"],
"syntaxes": [
"Packages/Swift/Syntaxes/Swift.tmLanguage",
"Packages/Decent Swift Syntax/Swift.sublime-syntax",
],
ahoppen marked this conversation as resolved.
Show resolved Hide resolved
"languageId": "swift"
},
{
"scopes": ["source.c"],
"syntaxes": ["Packages/C++/C.sublime-syntax"],
"languageId": "c"
},
{
"scopes": ["source.c++"],
"syntaxes": ["Packages/C++/C++.sublime-syntax"],
"languageId": "cpp"
},
{
"scopes": ["source.objc"],
"syntaxes": ["Packages/Objective-C/Objective-C.sublime-syntax"],
"languageId": "objective-c"
},
{
"scopes": ["source.objc++"],
"syntaxes": ["Packages/Objective-C/Objective-C++.sublime-syntax"],
"languageId": "objective-cpp"
},
]
}
}
"command": [
"sourcekit-lsp"
],
"selector": "source.c | source.c++ | source.objc | source.objc++ | source.swift"
}
```

Expand Down