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

Style: indentation of multiple-arity functions #95

Merged
merged 1 commit into from Dec 9, 2014
Merged

Style: indentation of multiple-arity functions #95

merged 1 commit into from Dec 9, 2014

Conversation

wjlroe
Copy link
Contributor

@wjlroe wjlroe commented Dec 3, 2014

This pull request adds guidelines on how to indent multiple-artity functions. The exact style here is to vertically align arguments and function body - as elements of any sequence would be indented; however this is up for discussion as there is no de facto agreement on how to indent these forms. Different tools have arbitrarily chosen different indentation styles and it would be good to reach an agreement on what most people prefer so that we can all conform to a consistent code style.

@amoe
Copy link

amoe commented Dec 3, 2014

This indentation style seems logical and attractive, IMO it should be merged. 👍

@Gonzih
Copy link
Contributor

Gonzih commented Dec 3, 2014

I think default style of using 2 spaces is much more intuitive in this case.
Also is simpler and probably supported by most of the tools by default.

@bbatsov
Copy link
Owner

bbatsov commented Dec 3, 2014

I think default style of using 2 spaces is much more intuitive in this case.

Why is it intuitive? It differs from the standard function structure. People have the tendency to confuse things that they're used to with things that are actually intuitive.

Also is simpler and probably supported by most of the tools by default.

A discussion in clojure-mode's issue tracker makes me believe that no tool except clojure-mode indented code this way (and clojure-mode doesn't do this anymore as well).

@Gonzih
Copy link
Contributor

Gonzih commented Dec 3, 2014

Ok, my bad about tools support.
If it's only specific to clojure mode then just merge it.

On 12/03/2014 03:38 PM, Bozhidar Batsov wrote:

I think default style of using 2 spaces is much more intuitive in
this case.

Why is it intuitive? It differs from the standard function structure.
People have the tendency to confuse things that they're used to with
things that are actually intuitive.

Also is simpler and probably supported by most of the tools by
default.

A discussion in clojure-mode's issue tracker makes me believe that no
tool except |clojure-mode| indented code this way (and |clojure-mode|
doesn't do this anymore as well).


Reply to this email directly or view it on GitHub
#95 (comment).

@zane
Copy link
Sponsor

zane commented Dec 3, 2014

In the absence of any clear reasons not to, matching the indentation style for single-arity functions seems like the right way to go here.

@@ -148,6 +148,28 @@ You can generate a PDF or an HTML copy of this guide using
(baz x)))
```

* <a name="indent-multiple-arity"></a>
Indent each arity form of a function vertically aligned with its
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of a function definition
arguments -> parameters

@bbatsov
Copy link
Owner

bbatsov commented Dec 7, 2014

I agree with the PR in general, but I've added a bit of stylistic comments.

@wjlroe
Copy link
Contributor Author

wjlroe commented Dec 9, 2014

@bbatsov I can rebase those commits if you'd like


;; bad - extra indentation
(defn foo
"I have many arities."
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many -> two

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops - fixed

@bbatsov
Copy link
Owner

bbatsov commented Dec 9, 2014

Yes, please.

@wjlroe
Copy link
Contributor Author

wjlroe commented Dec 9, 2014

Rebased

@bbatsov
Copy link
Owner

bbatsov commented Dec 9, 2014

P.S. A better commit message would be "Add a rule about the indentation of multi-arity functions"

@wjlroe
Copy link
Contributor Author

wjlroe commented Dec 9, 2014

Sure, happy to change it. I did a quick browse of the git history to see if I could divine a pattern

bbatsov added a commit that referenced this pull request Dec 9, 2014
Style: indentation of multiple-arity functions
@bbatsov bbatsov merged commit d11f342 into bbatsov:master Dec 9, 2014
@bbatsov
Copy link
Owner

bbatsov commented Dec 9, 2014

👍

@wjlroe wjlroe deleted the indentation-of-multiple-arity-forms branch December 9, 2014 15:00
@shaunlebron
Copy link
Contributor

shaunlebron commented Oct 16, 2015

This isn't representative of what people are doing in the core libraries. They indent the body after arguments in multi-arity functions. It's everywhere in clojure, clojurescript, core.async, tools.reader, and probably more.

In fact, clojure.pprint will indent one space after the arguments in multi-arity. Paste this code into the online clojure.pprint service to see it:

(defn symbol
  ([name]
   (if (symbol? name)
     name
     (let [idx (.indexOf name "/")]
       (if (== idx -1)
         (symbol nil name)
         (symbol (.substring name 0 idx)
                 (.substring name (inc idx) (. name -length)))))))
  ([ns name]
   (let [sym-str (if-not (nil? ns)
                   (str ns "/" name)
                   name)]
     (Symbol. ns name sym-str nil nil))))

after clojure.pprint:

(defn symbol
  ([name]
    (if (symbol? name)
      name
      (let [idx (.indexOf name "/")]
        (if (== idx -1)
          (symbol nil name)
          (symbol
            (.substring name 0 idx)
            (.substring name (inc idx) (. name -length)))))))
  ([ns name]
    (let [sym-str (if-not (nil? ns) (str ns "/" name) name)]
      (Symbol. ns name sym-str nil nil))))

@bbatsov
Copy link
Owner

bbatsov commented Oct 16, 2015

Much code in the wild was affected by an unfortunate bug in clojure-mode. Who knows, maybe even pprint was modelled after it. I'm one hundred percent certain that the current indentation is the most reasonable (and intuitive) way to indent such code, as it's consistent with the way single-arity functions are indented.

@marick
Copy link

marick commented Mar 11, 2016

I find the new style from clojure-mode makes multi-arity functions harder to read because the different arglist-body pairs don't stand out as well (especially for shorter ones).

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

Successfully merging this pull request may close these issues.

None yet

7 participants