borkdude / flycheck-clj-kondo

Emacs integration for clj-kondo via flycheck
94 stars 17 forks source link

Update clojure-ts-mode support #27

Closed p4v4n closed 6 months ago

p4v4n commented 6 months ago

This is a follow up PR to https://github.com/borkdude/flycheck-clj-kondo/pull/26.

borkdude commented 6 months ago

Thanks @p4v4n! CI is failing.

cc @kommen

bbatsov commented 6 months ago

I see why the CI is failing, but it seems to me it might be better to just depend on a explicit version of seq.el instead of relying on whatever's available in Emacs. The way I see it either we have to downgrade to 2.20 in flycheck or set some explicit dep here as well, so it can be fetched from GNU ELPA if needed.

bbatsov commented 6 months ago

Also - might be good to look into abstracting away the test code duplication for the different major modes.

p4v4n commented 6 months ago

There are 2 issues here.

  1. seq.el issue (Raised a ticket upstream as it will also likely affect other flycheck packages) and also
  2. I forgot that clojure-ts-mode requires emacs29 so I will need make a few more changes after seq.el fix.
p4v4n commented 6 months ago

The downside to using a explicit version of seq.el is that every downstream package(depending on flycheck) needs to keep track of what version of seq flycheck is depending on and update regularly whenever flycheck updates the version upstream.

kommen commented 6 months ago

I forgot that clojure-ts-mode requires emacs29

And also need to install (from source?) the clojure treesitter gramma, right? I think it is already more convenient but haven't tried/looked into how this works now.

p4v4n commented 6 months ago

And also need to install (from source?) the clojure treesitter gramma, right? I think it is already more convenient but haven't tried/looked into how this works now.

Luckily its already handled by clojure-ts-mode automatically with clojure-ts-ensure-grammars set to true by default. The tests are already passing on the emacs 29+ for this branch. https://github.com/borkdude/flycheck-clj-kondo/actions/runs/7945808126/job/21697659833

I think we only need to make sure that the clojure-ts-mode tests are not run when emacs-version < 29.1.

borkdude commented 6 months ago

Thanks!

p4v4n commented 6 months ago

Thanks @borkdude!

I was about to make a few more changes to this PR (like moving ts tests to a new file and other minor stuff). Will raise a separate PR later as it is not critical.