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

Update tree-sitter-swift #118

Conversation

DivineDominion
Copy link
Contributor

@DivineDominion DivineDominion commented Sep 8, 2022

The https://github.com/tree-sitter/tree-sitter-swift repository's README says the repo is abandoned and one should use https://github.com/alex-pinkus/tree-sitter-swift instead.

This PR changes the repository reference to match.

script/compile and script/test is working 👍

Edit: Noticed the issue; fixes #72

@jsmestad
Copy link
Collaborator

Looks like this is failing on this test

@alex-pinkus
Copy link

Those tests are failing because the grammar only supports emojis when compiled using tree-sitter version 0.20.5 or newer. I recall from previous discussions that emacs-tree-sitter uses 0.19.5, so I think that's expected.

@DivineDominion
Copy link
Contributor Author

I'm pretty new with all things tree-sitter, so any advise from you how to best resolve the situation? E.g. wait for emacs-tree-sitter to update the dependency before considering to merge this?

@alex-pinkus
Copy link

alex-pinkus commented Sep 20, 2022

Conversely, I'm pretty new to how the emacs-tree-sitter plugin works :)

Apart from not supporting emojis, the grammar behaves totally fine against older tree-sitter versions; there should be no issues merging this if there's some way to ignore those specific tests. It would still be a big improvement over what users are getting out-of-the-box today.

If it's not possible to ignore specific tests, I wonder if you could skip all of them; I run tests on every commit in CI and I still test highlight queries against 0.19.0, so the chance of breakage seems quite low.

This would also be a good reason to upgrade to a newer tree-sitter version, but I don't know what that entails :)

@konrad1977
Copy link

I've been using it for few weeks and it works great.

@alex-pinkus added some more keywords for highligtning. To get them themable we need to define them:

(defface tree-sitter-hl-face:repeat
  '((t :inherit tree-sitter-hl-face:keyword
       :foreground "#666bb2"))
  "Face for loops (for, in etc)."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:parameter
  '((t :inherit tree-sitter-hl-face:label
       :foreground "#666bb2"))
  "Face for parameters in function calls."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:conditional
  '((t :inherit tree-sitter-hl-face:property
       :foreground "#666bb2"))
  "Face for if/switch."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:include
  '((t :inherit tree-sitter-hl-face:property
       :foreground "#666bb2"))
  "Import/include face."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:boolean
  '((t :inherit tree-sitter-hl-face:property
       :foreground "#666bb2"))
  "Face for true/false."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:keyword.return
  '((t :inherit tree-sitter-hl-face:property
       :foreground "#666bb2"))
  "Face for 'return'."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:keyword.operator
  '((t :inherit tree-sitter-hl-face:property
       :foreground "#666bb2"))
  "Face for operator."
  :group 'tree-sitter-hl-faces)

(defface tree-sitter-hl-face:keyword.function
  '((t :inherit tree-sitter-hl-face:property
       :foreground "#666bb2"))
  "Face for 'func' keyword."
  :group 'tree-sitter-hl-faces)

I don't know where they belong, I've written my own "swift-mode" extensions that loads them the first time I enter a swift-buffer (along with setup of LSP (eglot). The foreground color is just a dummy though.

@shackra shackra requested a review from ubolonton September 20, 2022 19:39
@meain
Copy link
Member

meain commented Nov 27, 2022

We should probably update the tree-sitter version used in the CI. It requires a bit of rework of our test script. Let me see if I can get to that soon ish :D.

@jcs090218
Copy link
Member

Can you rebase this so the CI can test it? Thanks!

@DivineDominion DivineDominion force-pushed the ctietze/tree-sitter-swift-2022 branch from 9773482 to c46944a Compare September 4, 2023 07:31
@DivineDominion
Copy link
Contributor Author

@jcs090218 Done. Updates the submodule HEAD as well

@jcs090218
Copy link
Member

Thank you!

@jcs090218 jcs090218 merged commit dd0c886 into emacs-tree-sitter:master Sep 4, 2023
6 checks passed
@DivineDominion DivineDominion deleted the ctietze/tree-sitter-swift-2022 branch September 4, 2023 11:47
jcs090218 pushed a commit to jcs-PR/tree-sitter-langs that referenced this pull request Sep 4, 2023
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.

Update Swift language. Current is not the most active one.
6 participants