KEMBAR78
Actually check indentation for chained calls. by swang · Pull Request #285 · clutchski/coffeelint · GitHub
Skip to content
This repository was archived by the owner on Oct 19, 2021. It is now read-only.

Conversation

swang
Copy link
Collaborator

@swang swang commented Jun 8, 2014

The previous PR only addressed the INDENT tokens, this addresses the
issue of the chained calls being properly indented.

The previous PR only addressed the INDENT tokens, this addresses the
issue of the chained calls being properly indented.
@AsaAyers
Copy link
Collaborator

What do you think about adding an option indentChains optional/always/never?

Depending on your settings one of these might error.

something()
.one()
.two()

something()(
    .one()
    .two()

@swang
Copy link
Collaborator Author

swang commented Jun 13, 2014

I think optional/always/never may be a bit confusing. Does optional mean it does not care how you indent or just mean indenting properly is optional, and does never mean it never enforces indenting or that you should never indent?

For example if spacing was 2 and you set indentChain to 'optional' what does this return?

testa.b()
     .c()
     .d()

The user has indented to line the dots up with the rest of the chain. Does this violate an 'optional' lint?

I think there is essentially 4 types we have to worry about. Assuming indent spacing value is .

  • People who want strict enforcement of indents (indenting can only be indented exactly X from previous line, no more, no less)
  • People who want loose enforcement of indents (indenting can be a multiple of X from the previous line, I believe this is currently how the code for this PR handles indenting)
  • People who want no enforcement of indentation (so they could do something like my code example above, this is currently how the code in coffeelint handles indents in chains)
  • People who want to enforce no indenting in chains. (Your first code example)

There are probably a couple more styles people will have to deal with, but I think after this people should just write their own rule to enforce it.

I think personally I would go with strict, loose, none, and no_indents/no_indents_allowed(?) for these four types.

Also worried that maybe having too many options will add more complexity of the rule...

@AsaAyers
Copy link
Collaborator

I was only thinking of two styles, you either indent your chain the same way you indent a function or you don't. With optional it doesn't care either way, which seems to be the current behavior.

That's a good point about complexity. I'll just merge this as is and if someone wants to enforce one of those indention styles maybe it can be its own rule. I really don't think this already complex rule needs 4 more variations.

AsaAyers added a commit that referenced this pull request Jun 13, 2014
Actually check indentation for chained calls.
@AsaAyers AsaAyers merged commit aa129c1 into clutchski:master Jun 13, 2014
@swang swang deleted the check_indents_on_dots branch June 13, 2014 08:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants