Does not Formatter work well on single-line/multi-line block?

[at-grandpa]

(Please excuse my poor English.) I think there is no similar issues, but I'm sorry if there is duplicate.

$ crystal -v
Crystal 0.22.0 (2017-04-20) LLVM 4.0.0

---

I think crystal tool format does not work well in the following cases on block.

case1: single-line block with newline

crystal tool format does not work in the following cases.

a = [1, 2, 3].map { |i|
  i * i
}

# or

a = [1, 2, 3].map do |i|
  i * i
end

In both cases above, I expect the following because the block has single-line.

a = [1, 2, 3].map { |i| i * i }

case2: multi-line block with braces

In following case,

a = [1, 2, 3].map { |i|
  j = i + 1
  j * j
}

I think prefer do...end over {...} because the block has multi-line.

a = [1, 2, 3].map do |i|
  j = i + 1
  j * j
end

---

Is there the reason that crystal tool format dose not work well in the above cases?

[RX14]

Are you asking for the formatter to be opinionated on whether blocks span multiple lines or not, and whether they use do/end instead of {}? I'm for that change, but I don't think there's a robust way to figure out whether a 1-line block can be collapsed into an inline block. I would like to enforce do/end for non-inline blocks and {} for inline blocks in the formatter though.

[faultyserver]

See also "The Semantic Rule": https://github.com/bbatsov/ruby-style-guide/issues/162. The semantic rule says to use {} for any block that returns a value (e.g.,map, reduce, etc.), and do...end for all other blocks (each, etc.).

I don't wholeheartedly agree with the rule, but bbatsov's comment about enforcing such rules seems pertinent to me. Often times, the choice between do...end and {} is more than just the line-count or size, and I think there are enough users on each side that trying to enforce one style (however idiomatic) would annoy/alienate many of them.

There's also the issue of left-/right-association of blocks, where the difference is actually semantic, not just stylistic:

The difference between using do ... end and { ... } is that do ... end binds to the left-most call, while { ... } binds to the right-most call.

tl;dr: I don't think having the formatter modify the syntax used for the block is a good idea ({} vs do...end).

[RX14]

I'd much rather have a rule that can be enforced using crystal tool format than no rule like we have currently. I really enjoy using crystal tool format heavily as part of my development cycle and the stricter it is the better. gofmt has had a lot of acclaim for being really strict and i'm inclined to agree with them: I don't need to worry about formatting any more, it's not my job - it's the formatters - and what it says goes. This approach works best if everyone uses the strict formatter, and go seems to be achieving that goal. So i'd rather this kind of strictness was included in the crystal default formatter.

[asterite]

I'm not sure the formatter should be changed. In general the formatter only inserts whitespace and newlines, and removes more than two consecutive newlines to just two. But text is never changed. The few cases I can think now is removing extra () in calls like foo.bar(), or adding trailing commas in array literals that span multiple lines.

Imagine I want to write a block with multiple lines. I'll use | as the cursor:

[1, 2, 3].map do |x|
  a = x + 1
  |
end

I want to continue writing after that, but I have this OCD where I press Ctrl+S to save the code very often (in fact I tend to do that a lot ^_^). The formatter sees that there's only one line and changes it to:

[1, 2, 3].map { |x| a = x + 1 }

Arrrghh...! The formatter is getting in my way. I don't think this is nice.

This is the reason why the formatter only changes stuff that doesn't get in your way, and I think it should continue to behave like that.

I do, however, think that the formatter could change { ... } to do ... end if it sees that it spans multiple lines. So this:

[1, 2, 3].map { |x|
  x + 1
}

would automatically change to:

[1, 2, 3].map do |x|
  x + 1
end

Right now the formatter inserts newlines when you write do ... end. This:

[1, 2, 3].map do |x| x + 1 end

is changed to this:

[1, 2, 3].map do |x|
  x + 1
end

With this new rule the formatter will make sure that do ... end always appears in separate lines, and that { ... } can never appear in separate lines (they will be transformed to do ... end).

In general, I think this about code with { ... } and do ... end:

# good (simple inline { ... })
array.map { |x| x + 1 }

# bad ( { .. } with multiple lines, should use do/end or be in the same line, the formatter will do the former )
array.map { |x| 
  x + 1 
}

# good ( do/end in multiple lines, it doesn't bother that there's just one expression in it )
array.map do |x|
  x + 1
end

# good ( do/end in multiple lines with multiple expressions )
array.map do |x|
  a = x + 1
  a * a
end

# bad ( do/end in a single line, the formatter splits into multiple lines )
array.map do |x| x + 1 end

With the proposed change there will never be "bad" code.

What do you think?

[jhass]

I'm quite a fan of conveying additional intention via the choice of do/end vs {}. Similar to Weirich's rule I often use it to communicate whether I care about the callled methods return value or its side-effect. While this heavily correlates with single and multiline blocks in my experience, that's not always the case. Also I do format side-effect blocks with a single line over three lines usually, which has the benefit of giving them a higher visibility.

I would hate for the formatter to destroy such additional hints, and even make it impossible to add such nuances.

[faultyserver]

I want to continue writing after that, but I have this OCD where I press Ctrl+S to save the code very often (in fact I tend to do that a lot ^_^).

This is huge for me. I ended up disabling the formatter in Sublime less than and hour after the first time I wrote Crystal code because it was making changes that I didn't expect, and caused me to repeat space characters constantly. Worse yet, when I would repeatedly save in a medium-sized project, the formatter wouldn't have run the 2nd time by the time I "fixed" the file, so I would make a change and it would suddenly be reverted as soon as the second format run finished. Not a good experience, at all. _{I don't remember what these changes were, though. Likely space-before-bracket for blocks, like map{ becomes map {. Personal preference, but a strong enough one that I would disable formatting for.}

I would be okay with the formatter making more changes if it wasn't happening as I was writing the code. Or perhaps there could be different "levels" of formatting aggressiveness, so that only minor things like indentation change when I save in my editor, and then I can run a more aggressive formatter as a pre-commit hook or something.

[RX14]

@asterite that was pretty much exactly what I was proposing. I see the reasons for not transforming blocks from single/multi line now but I agree with you on the do/end and braces transformation.

[RX14]

@jhass I think formatting based on semantics is a bad idea. It's usually pretty obvious whether the block contains a side effect or an expression by reading it's contents. On top of that it means that I have to think about one more thing when writing my code. Formatting is pretty seperate from what I usually think about when writing code so it'd be hard to remember to context-switch into formatting mode every time I wrote a block. On top of that, this rule cannot be enforced automatically which makes it easy to miss when you've got it wrong in code reviews. Perhaps it's fine when you're working on a personal project but I don't know of any large project which uses this rule. In fact I had never heard of it before you linked it to me.

Most people who write ruby stick to the ruby style guide's recommendation to use {} for inline blocks and do/end for non-inline blocks. Most people who write crystal keep this tradition. If the formatter was changed to replace {} with do/end, it would help me immensely when refactoring single-line blocks into multi-line blocks because I could just press return at the beginning of the block contents, save, and the block would be formatted correctly and consistently. For that reason, I think that automating consistent formatting for the many is a better default than protecting semantic formatting for the few.

[asterite]

@faultyserver I just fixed a bug in the formatter where it would remove a newline if it had spaces in it. I switched to another computer where the text editor wasn't configured to automatically trim whitespace from newlines and it was driving me a bit crazy :-P. So maybe that's why it was annoying for you, I don't know. Let's see if in the next release you find the formatter a bit less annoying.

[RX14]

@asterite Oh thanks for this, in retrospect I think i've been hitting this bug and not realising it!

[at-grandpa]

Thank you for a lot of opinions.

I think case1 is transformed too much, so I agree with @asterite's proposal (https://github.com/crystal-lang/crystal/issues/4504#issuecomment-305985504).

However I would like to hear the opinions of others a little more, too.

[bcardiff]

FYI https://github.com/crystal-lang/crystal/pull/4517#issuecomment-310840350

Since #4517 was reverted either this should be demilestoned/reopen or have a other closing reason.

[RX14]

This should be reopened.

[sdogruyol]

The milestone is already past due. Can we relabel this please?