weavejester / cljfmt

A tool for formatting Clojure code
Eclipse Public License 1.0
1.11k stars 119 forks source link

Support var indentation metadata #49

Open bbatsov opened 8 years ago

bbatsov commented 8 years ago

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 @malabarba

Malabarba commented 8 years ago

Indeed. Would be nice if this could be supported by cljfmt too. If you decide to try, I'd be happy to help as well.

weavejester commented 8 years ago

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.

mitchelkuijpers commented 7 years ago

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 ^^

arrdem commented 7 years ago

@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: [2 4 2 4 2 4 2 4 ...] because he wanted result expressions to be on the next line and indented more deeply than the condition expressions.

I'd be happy to help if I can.

weavejester commented 7 years ago

I'm concerned that for instance @weavejester likes to write

(ns
  (:require
   []
   ...))

Actually I use:

(ns foo.bar
  (:require [foo.baz :as baz]
            [foo.quz :as quz]
            ...))

The current ns lines in cljfmt.core weren't written by me. I think I allowed that style because otherwise the lines would have been too long.

Neither indentation specifier provides a way to require linebreaks.

Well, because they're indentation specifiers, not linebreak specifiers. Automatically adding linebreaks is another problem.

arrdem commented 7 years ago

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.

Malabarba commented 7 years ago

@mitchelkuijpers Happy to help however I can.

PEZ commented 6 years ago

Is this still considered for implementation?

weavejester commented 6 years ago

Yes. I've just been too busy on other things to add this myself.

bbatsov commented 6 years ago

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.

PEZ commented 6 years ago

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?

mitchelkuijpers commented 6 years ago

@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.

bbatsov commented 6 years ago

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 :style/indent keys with it.

PEZ commented 6 years ago

Maybe I am misunderstanding something, but cljfmt uses rewrite-clj, if I recall correctly. And also if I recall correctly, rewrite-clj gives access to the metadata of the members of the AST (or whatever the structure is that it is using).

lread commented 3 years ago

@bbatsov's referenced spec is now here.

lread commented 3 years ago

@pez, @bbatsov et al, is there still interest for this?

I could explore, if there is interest. I was thinking about finally getting back to #36 and suppose this could be related. I suspect folks would choose some default for form alignment that they like but they'd want to override the default for specific forms via metadata.

vemv commented 3 years ago

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 'foo, try to resolve it, and then grab the meta out of the resolved var.

This is both fast and accurate. Accuracy matters because it's possible that one is performing alter-var-root! to add indent specs.

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.

bbatsov commented 3 years ago

@vemv I always had this as some form of translation - it would just be nice to have cljfmt pick up and convert the metadata indent specs automatically to its own format. Otherwise you often end up duplicating the same indentation rules.

camsaul commented 3 years ago

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 :style/indent specs to cljfmt specs from someone more familiar with both. Right now I have

(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 :style/indent specs (nothing fancy like [1 [:defn]] yet)

slipset commented 1 year ago

Just to chime in on this. This is becoming a noticeable problem for me, as clojure-lsp hands formatting off to cljfmt and we have some macros in our codebase which have indentation metadata on them. Nothing I can't live with, but I guess an AOL from me :)

borkdude commented 5 months ago

If I read through https://github.com/clojure-lsp/clojure-lsp/issues/1420 closely enough, it now uses clj-kondo to discover the :style/indent settings, then emits a cljfmt-compatible configuration and then calls cljfmt with that.

borkdude commented 5 months ago

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.