8
14
Fork
You've already forked zig-mode
1

Add tree-sitter enabled zig-ts-mode major mode #95

Open
nanzhong wants to merge 5 commits from nanzhong/tree-sitter into master
pull from: nanzhong/tree-sitter
merge into: ziglang:master
ziglang:master
nanzhong commented 2024年01月04日 04:34:17 +01:00 (Migrated from github.com)
Copy link

Addresses #89.

This PR refactors the existing zig-mode to derive from a new generic major mode zig-base-mode that holds common zig major mode configuration and then introduces a new tree-sitter enabled zig-ts-mode major mode that makes use of the zig tree-sitter grammar from https://github.com/maxxnino/tree-sitter-zig.

At the moment zig-ts-mode is pretty usable and supports tree-sitter powered syntax highlighting and indentation. It also derives common functionality like reformatter support, electric indent config, etc. from zig-base-mode. Two notable missing features are navigation and imenu integration (I don't use these much day to day -- I'm also very new to zig and have only been playing around with the language on the side -- so I have not yet implemented them).

I wanted to create this draft PR to outline and share the approach that I've taken. If it's directionally sound, I'm happy to do the remaining work needed to get the PR across the finish line (e.g. readme updates, tests).

I've configured it for use as follows (using elpaca and use-package):

(use-package zig-mode
 :elpaca (zig-mode :host github :repo "nanzhong/zig-mode" :branch "tree-sitter")
 :demand t
 :mode (("\\.zig\\'" . zig-ts-mode)
 ("\\.zon\\'" . zig-ts-mode))
 :init
 (add-to-list 'treesit-language-source-alist
 '(zig "https://github.com/maxxnino/tree-sitter-zig"))
 :hook (zig-ts-mode . (lambda ()
 (setq treesit-font-lock-level 4))))
Addresses #89. This PR refactors the existing `zig-mode` to derive from a new generic major mode `zig-base-mode` that holds common zig major mode configuration and then introduces a new tree-sitter enabled `zig-ts-mode` major mode that makes use of the zig tree-sitter grammar from https://github.com/maxxnino/tree-sitter-zig. At the moment `zig-ts-mode` is pretty usable and supports tree-sitter powered syntax highlighting and indentation. It also derives common functionality like reformatter support, electric indent config, etc. from `zig-base-mode`. Two notable missing features are navigation and imenu integration (I don't use these much day to day -- I'm also very new to zig and have only been playing around with the language on the side -- so I have not yet implemented them). I wanted to create this draft PR to outline and share the approach that I've taken. If it's directionally sound, I'm happy to do the remaining work needed to get the PR across the finish line (e.g. readme updates, tests). I've configured it for use as follows (using elpaca and use-package): ```emacs-lisp (use-package zig-mode :elpaca (zig-mode :host github :repo "nanzhong/zig-mode" :branch "tree-sitter") :demand t :mode (("\\.zig\\'" . zig-ts-mode) ("\\.zon\\'" . zig-ts-mode)) :init (add-to-list 'treesit-language-source-alist '(zig "https://github.com/maxxnino/tree-sitter-zig")) :hook (zig-ts-mode . (lambda () (setq treesit-font-lock-level 4)))) ```
Collaborator
Copy link

I genuinely think this should be a separated package, similar to the approach taken by numerous other major modes such as clojure-ts-mode, php-ts-mode, and so on.

I genuinely think this should be a separated package, similar to the approach taken by numerous other major modes such as `clojure-ts-mode`, `php-ts-mode`, and so on.
purcell commented 2024年03月01日 13:41:09 +01:00 (Migrated from github.com)
Copy link

Would definitely need to be a separate package, otherwise it couldn't be byte-compiled on the older Emacsen that this package supports.

Would definitely need to be a separate package, otherwise it couldn't be byte-compiled on the older Emacsen that this package supports.
Doomwhite commented 2024年03月08日 19:53:50 +01:00 (Migrated from github.com)
Copy link

I can't seem to be able to run eglot or lsp-mode with this major-mode...

I can't seem to be able to run eglot or lsp-mode with this major-mode...
Dev380 commented 2024年03月20日 18:14:22 +01:00 (Migrated from github.com)
Copy link

Eglot works for me, do you have logs you're willing to share?

Edit: it started breaking for me recently too, I'm not sure why. Eglot will say it can't detect the language server, and once it's given zls, it'll succeed but say it's managing 0 buffers in the project.

Eglot works for me, do you have logs you're willing to share? Edit: it started breaking for me recently too, I'm not sure why. Eglot will say it can't detect the language server, and once it's given zls, it'll succeed but say it's managing 0 buffers in the project.
joachimschmidt557 commented 2024年03月25日 18:04:48 +01:00 (Migrated from github.com)
Copy link

I agree with @jcs090218 and @purcell. At least as long as we support older Emacs versions which do not include tree sitter support by default, it is best to separate this into a different package/repository.

There is https://github.com/ryleelyman/zig-ts-mode already, but it would be nice to have an official package under the ziglang organization. @andrewrk can we get a zig-ts-mode repository in the ziglang organization?

I agree with @jcs090218 and @purcell. At least as long as we support older Emacs versions which do not include tree sitter support by default, it is best to separate this into a different package/repository. There is https://github.com/ryleelyman/zig-ts-mode already, but it would be nice to have an official package under the ziglang organization. @andrewrk can we get a `zig-ts-mode` repository in the ziglang organization?
Dev380 commented 2024年04月03日 16:46:10 +02:00 (Migrated from github.com)
Copy link

Would additional work be required to integrate the treesitter mode with this one? Or else commands that aren't specific to either the regular or tree sitter mode (such as the run command) would need to be duplicated across both. Kind of like what clojure is doing and IIRC Rustic mode used to have this too (where you can sub the treesitter mode in as a "backend" to the regular mode). Would a future official zig-ts-mode follow this architecture?

Would additional work be required to integrate the treesitter mode with this one? Or else commands that aren't specific to either the regular or tree sitter mode (such as the run command) would need to be duplicated across both. Kind of like what [clojure is doing](https://metaredux.com/posts/2023/03/12/clojure-mode-meets-tree-sitter.html) and IIRC Rustic mode used to have this too (where you can sub the treesitter mode in as a "backend" to the regular mode). Would a future official zig-ts-mode follow this architecture?
purcell commented 2024年04月03日 17:30:17 +02:00 (Migrated from github.com)
Copy link

There are two general patterns you'll see.

Common base mode

In this scheme you'd have an artificial zig-base-mode in zig-mode.el, as a parent mode for zig-mode. Then a separate zig-ts-mode package could use zig-base-mode as the parent mode too. Configuration like LSP etc. would then get hooked to zig-base-mode, while zig-mode would provide the non-treesitter font-locking and indentation.

This pattern is probably the best overall, but you won't see it in some cases because it requires the "classic" major mode author to add that artificial base mode.

Examples: python-mode/python-ts-mode, and other Emacs built-ins.

Completely independent ts mode

Here you'd have a zig-ts-mode package with no package dependency on zig-mode, and basically nothing in common. This pattern occurs most often in cases where the "base" package is heavyweight or opinionated, and the -ts-mode author wants to make a clean break from it.

Examples: nix-ts-mode, clojure-ts-mode, ocaml-ts-mode,


If maintained in the same github org, or even if not, I'd suggest the first pattern is preferable.

There are two general patterns you'll see. ### Common base mode In this scheme you'd have an artificial `zig-base-mode` in `zig-mode.el`, as a parent mode for `zig-mode`. Then a separate `zig-ts-mode` package could use `zig-base-mode` as the parent mode too. Configuration like LSP etc. would then get hooked to `zig-base-mode`, while `zig-mode` would provide the non-treesitter font-locking and indentation. This pattern is probably the best overall, but you won't see it in some cases because it requires the "classic" major mode author to add that artificial base mode. Examples: `python-mode`/`python-ts-mode`, and other Emacs built-ins. ### Completely independent ts mode Here you'd have a `zig-ts-mode` package with _no_ package dependency on `zig-mode`, and basically nothing in common. This pattern occurs most often in cases where the "base" package is heavyweight or opinionated, and the `-ts-mode` author wants to make a clean break from it. Examples: `nix-ts-mode`, `clojure-ts-mode`, `ocaml-ts-mode`, --- If maintained in the same github org, or even if not, I'd suggest the first pattern is preferable.
Ziqi-Yang commented 2024年09月24日 12:03:37 +02:00 (Migrated from github.com)
Copy link

Since this thread doesn't seems to have much process by months, and github:robbielyman/zig-ts-mode is still an half-completed work (only font lock is based on tree sitter). I made my own zig-ts-mode, which is on https://codeberg.org/meow_king/zig-ts-mode

Since this thread doesn't seems to have much process by months, and [github:robbielyman/zig-ts-mode](https://github.com/robbielyman/zig-ts-mode) is still an half-completed work (only font lock is based on tree sitter). I made my own `zig-ts-mode`, which is on https://codeberg.org/meow_king/zig-ts-mode
purcell commented 2024年09月24日 12:29:53 +02:00 (Migrated from github.com)
Copy link

I made my own zig-ts-mode

Looks good! I'd be happy to add this to MELPA directly pretty much as-is, but I have a couple of requests first:

  • Don't autoload the auto-mode-alist entry: it should be possible to install both this and zig-mode on machines without treesitter, without breakage. Treesitter and individual grammars don't show up as package dependencies, which makes things messy: every *-ts-mode package uses a different method for all this, and they all have disadvantages. The best compromise is to just tell the user to modify auto-mode-alist, or major-mode-remap-alist.
  • The variables like zig-ts-mode-font-lock-rules-definition are described as customisation, but I don't think those are useful extension points: if people improve these rules, they should send the improvements to you, not apply them locally. I'd just hard-code the zig-ts-mode-font-lock-rules without those vars.
> I made my own zig-ts-mode Looks good! I'd be happy to add this to MELPA directly pretty much as-is, but I have a couple of requests first: - Don't autoload the `auto-mode-alist` entry: it should be possible to install both this and `zig-mode` on machines without treesitter, without breakage. Treesitter and individual grammars don't show up as package dependencies, which makes things messy: every `*-ts-mode` package uses a different method for all this, and they all have disadvantages. The best compromise is to just tell the user to modify `auto-mode-alist`, or `major-mode-remap-alist`. - The variables like `zig-ts-mode-font-lock-rules-definition` are described as customisation, but I don't think those are useful extension points: if people improve these rules, they should send the improvements to you, not apply them locally. I'd just hard-code the `zig-ts-mode-font-lock-rules` without those vars.
purcell commented 2024年09月24日 12:31:08 +02:00 (Migrated from github.com)
Copy link

(Also the indent rules look surprisingly minimal, and at least one comment mentions rust-mode, probably copy-and-pasted)

(Also the indent rules look surprisingly minimal, and at least one comment mentions rust-mode, probably copy-and-pasted)
Ziqi-Yang commented 2024年09月24日 13:04:29 +02:00 (Migrated from github.com)
Copy link

Thank you for ur advice. They all make sense :). I've correct it according to ur advice.

Also the indent rules look surprisingly minimal,

Copied from the upstream zig tree sitter grammar query file for indentation. They should work for most cases


Edit:
Found some indentation issues. Working on it.

Thank you for ur advice. They all make sense :). I've correct it according to ur advice. > Also the indent rules look surprisingly minimal, Copied from the upstream zig tree sitter grammar query file for indentation. They should work for most cases --- Edit: Found some indentation issues. Working on it.
purcell commented 2024年09月24日 14:22:14 +02:00 (Migrated from github.com)
Copy link

Cool, added to MELPA — it should start building within a few hours. If you add a version tag to the repo, it'll show up in MELPA Stable too.

Cool, added to MELPA — it should start building within a few hours. If you add a version tag to the repo, it'll show up in MELPA Stable too.
purcell commented 2024年09月24日 14:25:19 +02:00 (Migrated from github.com)
Copy link

If in future everyone thinks it best, your repo could get moved to this GitHub org: we'd then need to adjust the MELPA recipe. Thanks for sharing your work!

If in future everyone thinks it best, your repo could get moved to this GitHub org: we'd then need to adjust the MELPA recipe. Thanks for sharing your work!
This pull request has changes conflicting with the target branch.
  • zig-mode.el
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin nanzhong/tree-sitter:nanzhong/tree-sitter
git switch nanzhong/tree-sitter
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
ziglang/zig-mode!95
Reference in a new issue
ziglang/zig-mode
No description provided.
Delete branch "nanzhong/tree-sitter"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?