cljfmt icon indicating copy to clipboard operation
cljfmt copied to clipboard

Support var indentation metadata

Open bbatsov opened this issue 9 years ago • 22 comments

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

bbatsov avatar Nov 29 '15 13:11 bbatsov

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.

Malabarba avatar Nov 30 '15 23:11 Malabarba

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.

weavejester avatar Nov 30 '15 23:11 weavejester

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 avatar Nov 17 '16 12:11 mitchelkuijpers

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

arrdem avatar Nov 17 '16 19:11 arrdem

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.

weavejester avatar Nov 17 '16 20:11 weavejester

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.

arrdem avatar Nov 17 '16 21:11 arrdem

@mitchelkuijpers Happy to help however I can.

Malabarba avatar Nov 21 '16 00:11 Malabarba

Is this still considered for implementation?

PEZ avatar Jun 17 '18 10:06 PEZ

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

weavejester avatar Jun 17 '18 14:06 weavejester

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.

bbatsov avatar Jun 17 '18 17:06 bbatsov

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 avatar Jun 17 '18 18:06 PEZ

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

mitchelkuijpers avatar Jun 25 '18 07:06 mitchelkuijpers

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.

bbatsov avatar Jun 25 '18 09:06 bbatsov

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

PEZ avatar Jun 26 '18 12:06 PEZ

@bbatsov's referenced spec is now here.

lread avatar Mar 15 '21 23:03 lread

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

lread avatar Apr 02 '21 16:04 lread

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.

vemv avatar Apr 02 '21 16:04 vemv

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

bbatsov avatar Apr 02 '21 19:04 bbatsov

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)

camsaul avatar May 19 '21 03:05 camsaul

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 :)

slipset avatar Dec 16 '22 12:12 slipset