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 swift language #2033

Merged
merged 4 commits into from
Apr 8, 2022
Merged

Add swift language #2033

merged 4 commits into from
Apr 8, 2022

Conversation

Dispersia
Copy link
Contributor

#1964

Was also planning on using this, so went ahead and implemented it. From that issue, I forked the current tree-sitter-swift and checked in the current build configs, until a better solution is implemented (if we just want to wait if they want to add a separate branch, that's fine too).

There are a few keywords that are in the highlights.scm that I didn't see in https://docs.helix-editor.com/themes.html#syntax-highlighting, so I didn't know how to handle them and just left them as is.

Finally, there is one issue in swift I can't seem to figure out.

image

when tabbing through, it's auto appending the parenthesis, but puts the cursor outside so you have to backspace. Is there a way to prevent this I didn't see?

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

this is looking good, just a few scopes need adjustment

runtime/queries/swift/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/swift/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/swift/highlights.scm Outdated Show resolved Hide resolved
@the-mikedavis
Copy link
Member

w.r.t. the lsp, I've seen similar behavior from erlang-ls, it will auto-complete arguments as well, so

maps:get| %% hit tab when cursor is on |
%% becomes
maps:get(Arg1, Arg2)|

I haven't dug into it further but I think it's behavior on the language server side. Maybe those language servers implement an outdated version of the spec.

@the-mikedavis
Copy link
Member

oh, also please run cargo xtask docgen and commit the results of that (will clear up the ❌ for the Docs ci)

@Dispersia
Copy link
Contributor Author

thanks, updated the PR, and that makes sense. I'm going to try to look into that issue further, but it auto completes significantly more (even inlines the parameter name) which I haven't seen other LSP's do

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

nice work, thanks!

@the-mikedavis the-mikedavis merged commit 9caf7c0 into helix-editor:master Apr 8, 2022
@the-mikedavis
Copy link
Member

@Dispersia for erlang-ls this issue has some discussion on it: erlang-ls/erlang_ls#772. From the discussion there it sounds like there's a "snippets" approach to doing autocomplete. I'll dig into this more but that might be a good hint for swift as well.

@the-mikedavis
Copy link
Member

Also would you mind adding a quick note to the new lsp wiki for sourcekit-lsp? https://github.com/helix-editor/helix/wiki/How-to-install-the-default-language-servers

@Dispersia
Copy link
Contributor Author

Added to the wiki, and appreciate the link, that will definitely help!

@Dispersia
Copy link
Contributor Author

Dispersia commented Apr 9, 2022

Finally understand why it does this. I don't know if helix supports snippets (or if it's a standardized thing, or it's a vscode thing), but what happens is in sourcekit-lsp they do this:
https://github.com/apple/sourcekit-lsp/blob/27956e6d6f029afcdeb3c151400d720270b40a2c/Sources/SourceKitLSP/Swift/CodeCompletion.swift#L205

Text returned from sourcekitd comes in as something such as append(<#T##contentsOf: #ContentsOf#String#>), however the client relays if it supports snippets or not. If it does support snippets, the line linked above replace the above content with append(contentsOf: ${1:Sequence}). When testing with VSCode, if that comes back in the edit response it puts the cursor over the text "Sequence" and has the entire word selected, letting you tab through changing each part (so multiple parameters would have ${2:... etc). If server says snippets aren't supported, it simply deletes the text except append(contentsOf: ) in this case, but it's just normal text so puts your cursor at the end, like it should.

I don't know if this is something that's want to be supported (trying to find if that's specific to vscode or it's part of an actual spec), but going to make the recommendation to remove all of it if snippets aren't supported to sourcekit-lsp's working group

Also created a post on their forums to see what they think about deleting everything besides the method if snippet is passed as false: https://forums.swift.org/t/additions-to-codecompletion/56649

@the-mikedavis
Copy link
Member

Ah makes sense! That's what I found with the Erlang LS as well (erlang-ls/erlang_ls#1263). There's a PR for snippets support in helix (#1178) but it seems like a pretty complicated feature and the PR appears to have paused.

IMO language servers should not auto-complete past the function name - not even the parens because that would mess up auto-pairs - because you have to edit backwards to deal with the generated stuff. Plus it kinda bumps with signature help (#1755)

@Dispersia
Copy link
Contributor Author

Dispersia commented Apr 9, 2022

Funny they both do the same thing, and we both came to the same conclusion of what should actually happen haha. Will wait to see their response, hopefully they agree 😄

EDIT: I just re-read your first response to the issue, I somehow completely missed that you had already stated it might have been due to snippets lol. /facepalm

@7ombie
Copy link
Contributor

7ombie commented Feb 3, 2024

Nice to see Swift supported (Xcode is horrible). However, the syntax highlighting is only partially functional. Types are only highlighted when they're used as type annotations, which is not how Swift is conventionally highlighted (in Xcode, at least, which uses the same logic). Take the following example:

let x: Float = 1
let y = Float(1)

The annotation (the Float on Line 1) is highlighted in Helix, but the Float on Line 2 isn't.

In the following example, MemoryLayout and Int should be highlighted as types, but are not highlighted at all:

let stride = MemoryLayout<Int>.stride

On the other hand, the type (SpaceInvader) is highlighted (as a type, despite not being an annotation) in this example:

let foo = SpaceInvader.staticMember

So, it's not even consistent.

The try keyword is highlighted as an operator (along with any ? or ! that follows it).

The ? and ! operators are generally highlighted inconsistently. It seems that ? is highlighted as if it's a type (except in the context of try?), while ! is not highlighted at all (it's not an operator, except in the context of try!).

I use minimal highlighting, so am not sure what else is missing, but noticed that enums do not reflect the style bound to type.enum (and there's no other enum scope). Not sure if that's intentional.

@Dispersia
Copy link
Contributor Author

Dispersia commented Feb 3, 2024

This is two years old, have you tried updating the comment and changing the config? I'm sure it supports all of the new syntax and has a lot of improvements, it looks fairly active and gets updated weekly (I haven't used swift since shortly after I added this) so I haven't had the need to update it

@7ombie
Copy link
Contributor

7ombie commented Feb 3, 2024

Updating which comment?

Changing which config? I'm using the Swift support that is included with Helix (with SourceKit LSP). I'm not sure what else I'm meant to use??

What gets updated weekly? Swift support?? It's not very mature, if that's the case.

If you're not using it, I guess it's not your problem anymore. I appreciate you taking the time to reply still. All the best.

@Dispersia
Copy link
Contributor Author

Dispersia commented Feb 5, 2024

Sorry my phone changed "commit" to "comment", but the grammars get updated weekly (which is everything from highlighting to when you hit enter how many spaces should be there, etc, which is supplied by tree-sitter):
https://github.com/alex-pinkus/tree-sitter-swift

you can see the current version helix is using is like a year and a half old:

source = { git = "https://github.com/alex-pinkus/tree-sitter-swift", rev = "77c6312c8438f4dbaa0350cec92b3d6dd3d74a66" }

If you want to try, its fairly simple to update these grammars, copy over their queries/{highlights|indents|locals|textobjects}.scm folder into the helixs queries/swift subfolder, update that commit id i linked in languages.toml, and then update the grammars like shown under https://docs.helix-editor.com/guides/index.html (it's just changing from like @Property to @function.property, and things like that)

should be fairly simple to open a PR for it if you're interesting in updating the grammars, and if you run into any issues feel free make an issue and we can discuss it

@7ombie
Copy link
Contributor

7ombie commented Feb 6, 2024

Thank you, @Dispersia. I see. I can't commit to doing it right now, but I am keen to improve Swift support, and to begin contributing to Helix. I don't know Rust, but could help with the Swift grammar etc.

should be fairly simple to open a PR for it if you're interesting in updating the grammars, and if you run into any issues feel free make an issue and we can discuss it

Will do. Thanks again. All the best.

@7ombie
Copy link
Contributor

7ombie commented Feb 7, 2024

I'm just looking into this a bit more. I think I get most of it, but am unsure on what to commit to (to get the updated ID). I installed Helix with brew, so the files are in /opt/homebrew/Cellar/helix/23.10/, and I don't want to confuse Homebrew (which uses git too).

I can reinstall Helix manually, and build it from source, but even then, I'm not sure if I'm supposed to commit to Helix or to a Swift-specific repo (after I update the scm files)??

I'd appreciate any guidance. Thanks again.

@Dispersia
Copy link
Contributor Author

the updated id is just whatever the latest commit on their branch is, so https://github.com/alex-pinkus/tree-sitter-swift latest (as of me posting this message) is 1c586339fb00014b23d6933f2cc32b588a226f3b

you'll be commiting to helix, with changes to this directory: https://github.com/helix-editor/helix/tree/master/runtime/queries/swift with changes from theirs for the files i had listed above

and ya, you will need to build it locally, as you will need to update the languages.toml, which is built into the executable (but you wont touch any rust code, just configurations)

@7ombie
Copy link
Contributor

7ombie commented Feb 7, 2024

Thank you, @Dispersia. Much appreciated. I'll give it a go.

@7ombie
Copy link
Contributor

7ombie commented Feb 8, 2024

Sorry to be a pain. I'm a bit stuck.

I cloned TreeSitter and Helix, set HELIX_DEFAULT_RUNTIME to the runtime directory inside the repo, then built Helix from source. That seemed to work fine. Helix found its grammars etc, as well my ~/.config/helix (which does not contain a runtime directory).

I then edited .../runtime/queries/swift/highlights.scm within the Helix repo, making a few changes, based on the tree-sitter version (TreeSitter's highlights.scm hasn't evolved much, and I figured I could focus on the other SCM files later). I also updated languages.toml to point to the newest commit hash for TreeSitter (which was still the one you posted).

When I tried to rebuild Helix (using cargo install --path helix-term --locked) it failed with this message:

  Failure 1/1: swift Failed to compare source and binary timestamps

  --- stderr
  thread 'main' panicked at 'Failed to compile tree-sitter grammars: 1 grammars failed to build', helix-term/build.rs:7:14

Sorry to keep bothering you @Dispersia. Is there somewhere better I can ask for help with this? Thanks again.

@7ombie
Copy link
Contributor

7ombie commented Feb 8, 2024

I'll open a new issue, @Dispersia. It's not really fair to expect you to onboard me (no good deed goes unpunished, ay). Best wishes. Thanks again. You've been awesome.

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.

None yet

4 participants