-
Notifications
You must be signed in to change notification settings - Fork 120
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
Support var indentation metadata #49
Comments
Indeed. Would be nice if this could be supported by cljfmt too. |
This seems like a good idea. I think the indentation spec could potentially replace cljfmt's own, less well thought out syntax. I don't think there's anything I'm doing that isn't covered by the linked spec. |
I would like to take a stab at this @weavejester Would you still consider this? @Malabarba I think I'll take you up on the help ^^ |
@mitchelkuijpers awesome! Thanks for taking this up, I glanced at it a few weeks ago and didn't get anywhere. As you work on this, I'd keep a critical eye on both indentation spec formalisms. I'm concerned that for instance @weavejester likes to write (ns
(:require
[]
...)) Neither indentation specifier provides a way to require linebreaks. Also as far as I'm able to tell, neither indentation specifier is able to support repetitions of term(s). I haven't been able to find the particular thread with some quick googling, but a fellow came along who was using the following spec for cond: I'd be happy to help if I can. |
Actually I use: (ns foo.bar
(:require [foo.baz :as baz]
[foo.quz :as quz]
...)) The current
Well, because they're indentation specifiers, not linebreak specifiers. Automatically adding linebreaks is another problem. |
Apologies for casting stones, my point was more that neither spec provides line break specifications, nor term repetition nor patterns and all of those both seem to be stumbling blocks which someone's gonna want to fix eventually. |
@mitchelkuijpers Happy to help however I can. |
Is this still considered for implementation? |
Yes. I've just been too busy on other things to add this myself. |
Same here. I planned to work on this myself, but never found the time. I still think that this is going to be an awesome addition to cljfmt and a big step towards wider adoption of a common indentation spec. |
Great! I too think it would be a big step away from the indent war between different editors and tools. Are you still eager to help, @mitchelkuijpers and @Malabarba? |
@PEZ Sure, I tried starting on this but what was hard is that you can run cljfmt outside of the clojure process. So got stuck on finding the var metadata to define the indentation on symbols. If you have any ideas I would be happy to take this on. |
I think you can simply use a parser to extract the metadata https://github.com/clojure/tools.analyzer Probably given the limited scope of this you can just use https://clojure.github.io/clojure/clojure.walk-api.html and simply look for the relevant |
Maybe I am misunderstanding something, but |
@bbatsov's referenced spec is now here. |
@PEZ, @bbatsov et al, is there still interest for this? I could explore, if there is interest. |
FWIW, I've used a cider->cjlfmt indent spec translation for the past two years without any noticeable issue My approach is basically for a given symbol This is both fast and accurate. Accuracy matters because it's possible that one is performing Because 99% of things having an indent spec are defmacros, most likely those macros are being processed with JVM clojure and therefore JVM clojure is available and usable, even if targeting clojurescript. My spec translation defn is as follows: (defn cider-indent->cljfmt-indent [{cider-indent :style/indent
cljfmt-indent :style.cljfmt/indent
cljfmt-type :style.cljfmt/type}]
(or cljfmt-indent
(and (number? cider-indent) [[(or cljfmt-type :block)
cider-indent]])
(and (#{:defn} cider-indent) [[:inner 0]])
nil)) And its semantics are described in detail here: https://github.com/nedap/formatting-stack/wiki/Indentation-examples Personally I haven't bothered proposing something here because cljfmt has its own format, so proposing a change in format (i.e., to CIDER's) could be a bit invasive. Whereas a translation layer needs no coordination :) If you think there's something useful that can be extracted from my approach, LMK - I could contribute a PR. |
@vemv I always had this as some form of translation - it would just be nice to have |
I have a working demo for this in #230. I pushed a working deps.edn demo to https://github.com/camsaul/cljfmt. You can try it locally with clojure -Sdeps \
'{:deps {camsaul/cljfmt {:git/url "https://github.com/camsaul/cljfmt", :sha "ec3884db9b0961c59fd74972c1ca4594f0ab477f"}}}' \
-M -m cljfmt.main [check|fix] I could use a little help with translating (defn- style-indent-spec->cljfmt-spec [spec]
(cond
(number? spec)
[[:block spec]]
(= spec :defn)
[[:inner 0]]
:else
(binding [*out* *err*]
(println "Don't know how to convert" (pr-str spec) "to a cljfmt spec")
nil))) which only handles a basic |
Just to chime in on this. This is becoming a noticeable problem for me, as |
If I read through clojure-lsp/clojure-lsp#1420 closely enough, it now uses clj-kondo to discover the |
If people want this feature without clojure-lsp, I can imagine a small extra command line tool that combines clj-kondo and cljfmt could also work. |
We've come up with some indentation spec for CIDER's dynamic indentation functionality, we consider fairly flexible and not CIDER-specific at all. The details are here.
It'd be nice if
cljfmt
(and other tools in general) respected indentation settings provided via such metadata. //cc @MalabarbaThe text was updated successfully, but these errors were encountered: