Unsure about correct alignment of block ends

[lawliet89]

Given the following code:

    params = default_options.merge(options)
              .delete_if { |k, v| v.nil? }
              .each_with_object({}) do |(k, v), new_hash|
                new_hash[k.to_s] = v.to_s
              end

Rubocop complains with the BlockAlignment cop and wants me to align it as such:

    params = default_options.merge(options)
              .delete_if { |k, v| v.nil? }
              .each_with_object({}) do |(k, v), new_hash|
                new_hash[k.to_s] = v.to_s
    end

Is this intended behaviour? I find it's not actually better style to align it as such.

[bbatsov]

Have a look here #393

[lawliet89]

So the solution is to not use multi-line chaining, or disable the cop?

[bbatsov]

At the moment - that's pretty much it. This is very hard to get right in all the cases. However, I write a lot of Ruby code (obviously) and I don't have a problem with the current state of the cop.

If someone manages to improve the cop to reliably support all sensible scenarios that'd be great, but it's unlikely.

[lawliet89]

Right, I'll disable it then. Thanks.

[bbatsov]

@jonas054 @yujinakayama Looking at this particular recurring issue, maybe we should report an offence only in situations when there's no ambiguity. While I'm obviously against writing code like the one in the example - a lot of people are not. Other suggestions are welcome!

[yujinakayama]

maybe we should report an offence only in situations when there's no ambiguity.

👍

And probably we should add a cop to check multi-line chaining with blocks?

[jonas054]

Good suggestions @bbatsov and @yujinakayama! We'll make the BlockAlignment cop just skip blocks that are part of a multi-line chain, and thus allow any indentation of end there. And also add a new cop. I'll get started on these things if you don't mind.

[bbatsov]

Sure. 

—

Cheers,
Bozhidar

On Fri, Aug 23, 2013 at 7:15 PM, Jonas Arvidsson notifications@github.com
wrote:

Good suggestions @bbatsov and @yujinakayama! We'll make the BlockAlignment cop just skip blocks that are part of a multi-line chain, and thus allow any indentation of end there. And also add a new cop. I'll get started on these things if you don't mind.

Reply to this email directly or view it on GitHub:
#447 (comment)

[jonas054]

In #338 @edzhelyov talked about checking if end is aligned with the start of the line where do is, and I think I have finally come around to embracing that idea. If we just let BlockAlignment accept that style of end alignment in addition to the current one, we'll be golden.

[jonas054]

Updated specs with more code examples and comments about where they come from. Also changed to cop.messages.