Skip to content
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

Open
bbatsov opened this issue Nov 29, 2015 · 22 comments
Open

Support var indentation metadata #49

bbatsov opened this issue Nov 29, 2015 · 22 comments

Comments

@bbatsov
Copy link
Contributor

bbatsov commented Nov 29, 2015

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
Copy link

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
Copy link
Owner

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
Copy link

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
Copy link
Contributor

arrdem commented Nov 17, 2016

@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
Copy link
Owner

weavejester commented Nov 17, 2016

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
Copy link
Contributor

arrdem commented Nov 17, 2016

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
Copy link

@mitchelkuijpers Happy to help however I can.

@PEZ
Copy link
Contributor

PEZ commented Jun 17, 2018

Is this still considered for implementation?

@weavejester
Copy link
Owner

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

@bbatsov
Copy link
Contributor Author

bbatsov commented Jun 17, 2018

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
Copy link
Contributor

PEZ commented Jun 17, 2018

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
Copy link

@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
Copy link
Contributor Author

bbatsov commented Jun 25, 2018

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
Copy link
Contributor

PEZ commented Jun 26, 2018

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
Copy link
Contributor

lread commented Mar 15, 2021

@bbatsov's referenced spec is now here.

@lread
Copy link
Contributor

lread commented Apr 2, 2021

@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
Copy link
Contributor

vemv commented Apr 2, 2021

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
Copy link
Contributor Author

bbatsov commented Apr 2, 2021

@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
Copy link
Contributor

camsaul commented May 19, 2021

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
Copy link

slipset commented Dec 16, 2022

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
Copy link
Contributor

If I read through clojure-lsp/clojure-lsp#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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests