Configuration option for two space list indentation

[miridius]

Closes #323

Updated approach:

Adds a new config option :function-arguments-indentation with 3 possible values:

1. :community - the default, and same as current behaviour. Follows the community style recommendation to indent function/macro arguments by a single space when there are no arguments on the same line as the function name.
2. :cursive copies Cursive's behaviour when it has "one space list indent" disabled (which is the default). Indents lists by 2 spaces when there is only one element on the first line, unless the first element (not counting metadata) is a data structure literal.
3. :zprint copies zprint's default behaviour. Uses two space indentation if a list starts with a symbol or a keyword and has no arguments on the same line as that symbol/keyword.

Original approach:

Adds two new config options:

1. :one-space-list-indent? - whether cljfmt should follow the community style guide's rule to indent function arguments by one space (true) or use two spaces (false). Defaults to true
2. :one-space-list-indent-tags - Only has an effect if :one-space-list-indent? is false. For lists that aren't plain function calls, we sometimes want to use one space indentation. A good example is multi-arity functions, which both Cursive and zprint indent by one space even when two space indentation is enabled. For most other types of elements, Cursive and zprint disagree, so this config makes cljfmt flexible enough to match either of them. Defaults to Cursive style, which is that any list starting with a data structure has one space indent (and if there is a meta literal, it checks the type of element the meta is applied to). The tests also show how to match zprint style.

I've added tests and documented both in the README, and added a CLI flag for the first one. I don't think it's worth trying to make a CLI flag for the latter but happy to be told otherwise. I've also tried using this on a large codebase formatted by IntelliJ to check for discrepancies.

I'm not sure if this config approach is necessarily the way you want to go with it, e.g. perhaps you want to group both options under a single key. Feedback is welcome :)

[weavejester]

Thanks for the PR.

Let's invert the option, I think, since it's often more intuitive to enable optional settings, rather than to disable them. So instead, :two-space-list-indent? which defaults to false.

Regarding :one-space-list-indent-tags, I don't like the idea of exposing the internal tag names rewrite-clj uses. Perhaps instead we have an option like: :ignore-two-space-list-indent-when-starting-with-collection?. Can you think of a more concise name without losing clarify?

[miridius]

What if we just have a single flag with 3 preset values? something like :function-arguments-indentation with 3 possible styles (names TBD):

• :community (the default), always does 1 space
• :cursive, 2 spaces except when the list starts with a collection or meta + collection
• :zprint, 2 spaces only when the list starts with a symbol or keyword

[miridius]

referring directly to the other projects is probably not ideal as then that adds a maintenance burden to make sure we stay in sync. So maybe some more semantic names

• :one-space
• :two-spaces-when-list-starts-with-symbol-or-keyword
• :two-spaces-unless-list-starts-with-collection

[weavejester]

That's a tough decision. I agree that descriptive names would be less confusing if zprint or cursive changed their defaults. On the other hand, the shorter names can be used more easily in arguments, and :one-space isn't an full description of the "classic" lisp formatting style, as the indentation changes if there is more than one element on the first line.

What about this: we use :community, :cursive and :zprint, and if these change in future, we version them. So for example if zprint 1.5 changed the format, we might have :zprint and :zprint-1.5+. Of course, this may never happen.

This also allows for potentially more complex indentation rules that cannot easily be boiled down to a short description, as in the case of the community standard.

Thoughts?

[miridius]

I like that plan :) I've pushed a new commit with the change (will squash later after the PR is ready to merge)

[miridius]

politely bumping this @weavejester

[weavejester]

Thanks for the bump, I managed to miss the notification before. As part of the tests, one thing I notice as missing is:

(foo bar
baz)

Which in the community standards is formatted as:

(foo bar
     baz)

How does this compare to zprint and Cursive?

[miridius]

Cursive & Zprint would do the same thing in that scenario. We're only applying this new config when there are no arguments on the same line as the function name. Having a test for that would be a good idea though!

I'm not completely convinced that the way I added new tests makes sense. What I actually did was changed the default setting to :cursive, ran the tests, found every test that failed and copy-pasted it to a new test with the updated indentation, then changed the default back again. So that's why there didn't end up being any tests for your example. There's also a lot more tests for :cursive than there are for :zprint, but I also didn't want to make the whole test suite 3x longer. I wonder if there's some clever way around that using are or something?

[weavejester]

I think that's a reasonable way to add new tests. However, I'd suggest adding the case with one argument on the same line, just to ensure that the original behavior remains the same in that case, and to prevent any future regressions.

Otherwise, everything else looks good.

[miridius]

Done 👍

[miridius]

bump 🙂

[weavejester]

Sorry for the delay. This all looks good! Can you ensure that all lines are under 80 characters in length - I think there are a couple that are a little too long and just need to be broken up onto two lines - and then if you squash your commits down we should all be ready to merge and cut a new release. Thanks for all your work on this.

[miridius]

done and done 👍

[weavejester]

Thanks! Could you change the commit message to:

Add support for zprint/cursive indentation

Add a new configuration option, :function-arguments-indentation, that
can be one of: :community, :cursive or :zprint. This controls the
behavior described in: https://guide.clojure.style/#one-space-indent

When set to :community (the default) it follows the Clojure style guide.
When set to :cursive or :zprint, it follows the conventions used by
those tools.

Also add a --function-arguments-indentation option to update this at the
command line.

cljfmt follows the seven rules of a great git commit message for its commit messages. Once that's done, I'll merge and release.

[miridius]

Sure thing, done!

[miridius]

bump 😉

[weavejester]

Merged! Sorry for the delay.