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 · 14 comments

Comments

Projects
None yet
6 participants
@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

This comment has been minimized.

Copy link

Malabarba commented Nov 30, 2015

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

This comment has been minimized.

Copy link
Owner

weavejester commented Nov 30, 2015

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

This comment has been minimized.

Copy link

mitchelkuijpers commented Nov 17, 2016

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link

Malabarba commented Nov 21, 2016

@mitchelkuijpers Happy to help however I can.

@PEZ

This comment has been minimized.

Copy link
Contributor

PEZ commented Jun 17, 2018

Is this still considered for implementation?

@weavejester

This comment has been minimized.

Copy link
Owner

weavejester commented Jun 17, 2018

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

@bbatsov

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link

mitchelkuijpers commented Jun 25, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment