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

Format: indent multiline call chaining with 4 spaces #5234

Conversation

makenowjust
Copy link
Contributor

Closes #5195

See #5195 (comment)

@yxhuvud
Copy link
Contributor

yxhuvud commented Nov 3, 2017

I just wonder how easy it will be to make this kind of exception in the editors. Not all use the builtin formatter by default.

@RX14
Copy link
Contributor

RX14 commented Nov 3, 2017

@yxhuvud they should. But I think this rule is easy to apply while typing too. It's just when the line starts with a . mostly.

@makenowjust
Copy link
Contributor Author

ping

@ysbaddaden
Copy link
Contributor

I quote and support @asterite : "I think the formatter should always use 2 spaces to indent."

This is what I always use: 2 spaces.

  • chained calls on multiple lines already have the leading '.' to give the chained information;
  • 4 spaces disrupt the usual 2 spaces indentation, despite having nothing peculiar to outline;
  • it eats 2 more chars on the line (which I keep under 80).

This is unwelcomed, and one more reason to not use the formatter.

@makenowjust
Copy link
Contributor Author

Hm, @ysbaddaden's opinion makes sense. Do you have a counter, @RX14, @straight-shoota or any one?

@RX14
Copy link
Contributor

RX14 commented Nov 12, 2017

@ysbaddaden it's not a big deal, but I outlined by argument here. If everyone else agrees that an indent of 2 is fine, let's merge this PR with that.

@makenowjust makenowjust force-pushed the fix/crystal-format/multiline-call-4-space-indent branch from 03d1879 to 4ab32d1 Compare November 25, 2017 04:56
@makenowjust
Copy link
Contributor Author

@RX14 changed indent size as 2 spaces.

@makenowjust
Copy link
Contributor Author

ping

Copy link
Member

@asterite asterite left a comment

Choose a reason for hiding this comment

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

So this basically removes the feature of the formatter aligning calls to the dot, right? If so, I think that's fine. More consistent (always 2 spaces for indent), less discussions.

@RX14 RX14 force-pushed the fix/crystal-format/multiline-call-4-space-indent branch from 4ab32d1 to fe8d3b4 Compare January 2, 2018 15:57
@RX14 RX14 merged commit 84fa9ec into crystal-lang:master Jan 2, 2018
@RX14 RX14 added this to the Next milestone Jan 2, 2018
@makenowjust makenowjust deleted the fix/crystal-format/multiline-call-4-space-indent branch January 2, 2018 16:01
makenowjust added a commit to makenowjust/crystal that referenced this pull request Oct 12, 2019
Fixed crystal-lang#8197

Also remove @multiline_call_indent trick introduced by crystal-lang#5234.
It was originally introduced for dot alignment as @dot_column.
However it is removed and this trick can be implemented without any trick.
asterite pushed a commit that referenced this pull request Oct 13, 2019
Fixed #8197

Also remove @multiline_call_indent trick introduced by #5234.
It was originally introduced for dot alignment as @dot_column.
However it is removed and this trick can be implemented without any trick.
@jhass jhass mentioned this pull request Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants