renzmann / treesit-auto

Automatic installation, usage, and fallback for tree-sitter major modes in Emacs 29
GNU General Public License v3.0
369 stars 28 forks source link

fix(clojure): add major mode remappings for clojurec and clojurescript #54

Closed jasonjckn closed 9 months ago

jasonjckn commented 11 months ago

fix(clojure): add major mode remappings for clojurec and clojurescript

renzmann commented 10 months ago

Hi @jasonjckn, thanks for the contribution! Do you mind pulling from the most recent copy of the main branch and adding the :ext keyword for each of the two recipes you've added? As soon as those are in, I will merge this PR.

jasonjckn commented 10 months ago

@renzmann I rebased and pushed, and i think clojure-ts-mode changed a little bit since we last talked, everything should be in order now https://github.com/jasonjckn/treesit-auto/commit/a009c0b48d02c03779667ec9426906806fa7c2de

renzmann commented 10 months ago

I've been playing with this a bit, and I'm wondering if these two extra recipes are needed? Assuming clojure-ts-mode is installed and I've got the grammar compiled, Emacs is automatically switching between ClojureC[TS], ClojureScript[TS], and Clojure[TS], depending on the file extension. So the remaining question is: do we simply want to remap clojurescript-mode and clojurec-mode to clojure-ts-mode so that we get prompts for installation? If so, we might be able to do this a bit more succinctly within the clojure recipe by itself:

(make-treesit-auto-recipe
  :lang 'clojure
  :ts-mode 'clojure-ts-mode
  :remap '(clojure-mode clojurescript-mode clojurec-mode)
  :url "https://github.com/sogaiu/tree-sitter-clojure"
  :ext "\\.cljc?s?\\'")

LMK your thoughts and if this still works in your setup (I'm not a Clojure dev, so it's a bit hard for me to test thoroughly)

jasonjckn commented 10 months ago

I actually didn't notice this before, or maybe it was added recently, it looks like the author of clojure-ts-mode is configuring a lot of 'treesit-auto' like settings, does this cover everything

image

Does that make my PR obsolete? They're also adding more languages, clojure dart, .cljd, and jank lang .jank, babashka *.bb.

we might be able to do this a bit more succinctly within the clojure recipe by itself:

The condensed form looks fine to me, and I tested it a bit.

renzmann commented 10 months ago

Without looking too closely... yeah that chunk is basically doing some of what this package would handle - setting major-mode-remap-alist if clojure-mode is already installed, and handling auto-mode-alist in the case someone calls treesit-auto-add-to-auto-mode-alist in their configuration.

It's still useful to include clojure-ts-mode in the recipes here, though, so that you can download and compile the grammar without too much overhead, even though it looks like clojure-ts-mode will handle a lot of the alist stuff. So, altogether, if we adjust this PR to just swap the clojure recipe to the one I suggested above I'll be happy to merge it in.

jasonjckn commented 9 months ago

Ok, I pushed again, i also added *.cljd extension for clojure dart