Block indent with assignment on previous line

As noted by @kballenegger in issue #249, vim-ruby currently indents some cases of blocks wrong. On my machine, the indenting works like so:

(@superclass_cache ||= {})[service_name.to_sym] ||=
  Class.new(InternalServiceController) do
  singleton_class.class_eval { alias :new :old_new }
  end.tap {|k| k.const_set(:SERVICE_NAME, service_name) }

A simpler example would be:

@one ||=
  two do
  "three"
  end

The problem here is that the indent of the block content line considers the MSL of line 2, which is the ||= line, while the end considers the two do line. The correct indent should probably be:

@one ||=
  two do
    "three"
  end

@kballenegger, I've pushed a commit that should fix this issue, could you take a look and let me know if that's the case?

This seems to work for the most part. I've only found a couple bugs with pipelines.

This is the new automatic indenting:

myClass ||=
  Class.new do
    yes it_works!

    somevar = case one
              when two then three
              when four then five
              end

    blah = some \
      .long \
      .pipeline \
      .of do
      stuff
    end

    same
      .with.or
      .without do
      backslash
    end

    a = more.
      traditionally.
      style do
        pipeline
      end.
      is.styled.
      correctly

      but.does.not.reset.correctly.after
  end

Basically, this case is fixed (yay!):

blah ||=
  begin
    ...
  end

But pipelines do weird things.

With leading periods (my preferred style):

blah
  .blah do
  "This is indented wrong!"
end

The last two lines lost their indent.

With trailing periods:

blah.
  blah do
    "This is indented correctly! :)"
  end

  but "everything that follows is two spaces to the right, incorrectly."

Based on your last comment in #249 I think you're already aware of this, thought.

I wasn't aware, no. My comment in #249 is regarding the let g:ruby_begin_continuation_style = "blah" proposal.

These two potential problems that you've discovered don't seem to be related to the change, though. Checking out one commit before master has the same result. That said, I guess we should discuss them here before considering opening up new issues.

First off, this:

blah
  .blah do
  "This is indented wrong!"
end

blah.
  blah do
  "This is indented wrong!"
end

I don't think this is a bug. Consider:

foo(blah,
    blah) do
  "This is indented wrong!"
end

The top line ends with a comma, so we consider it a continuation. But in this case, it makes a lot of sense to just consider the top line broken up and align the rest of the body according to the "parent" line. This works the same way for things like:

def foo
  opts.on('--coordinator host=HOST[,port=PORT]',
          'Specify the HOST and the PORT of the coordinator') do |str|
    h = sub_opts_to_hash(str)
    puts h
  end
end

if foo || bar ||
    bong &&
    baz || bing
  puts "foo"
end

foo,
  bar = {
  :bar => {
    :foo => { 'bar' => 'baz' },
    :one => 'two',
    :three => 'four'
  }
}

You can find these examples in the specs, and I'm pretty sure we've had open issues for all of them. The general rule is, continuations are line-based, and block indentation respects the "main" line that these continuations fit into.

The assignment case is the odd one out:

foo =
  if bar
    baz
  end

I'm not sure why this was implemented like that, it was done before my time. I guess people like this (I ended up using it as well), but it's definitely a special case. The bug that this issue is about is not whether it should be implemented, the bug was simply that this works in this way for if-clauses and other structures, but not for do-blocks:

foo =
  bar do
    baz
  end

This simply unifies the two cases.

But I don't think we should change the handling of all the other cases (commas, dots, various operators).

This, however, seems very wrong:

a = more.
  traditionally.
  style do
    pipeline
  end.
  is.styled.
  correctly

  but.does.not.reset.correctly.after

Then again, I'm not even sure what the right indenting should be in this case... I guess something like this:

a = more.
  traditionally.
  style do
  pipeline
end.
  is.styled.
  correctly

but.does.not.reset.correctly.after

For an example that actually does something:

[1, 2, 3].select do |i|
  i.odd?
end.
  map do |i|
    i * i
  end

Though, honestly, I'd rather write this like so:

[1, 2, 3].select do |i|
  i.odd?
end.map do |i|
  i * i
end

I'll see what I can figure out about it. It seems very much like and edge case (at some point, you really might want to consider extracting variables and/or using the second form above), so if I can't find a sensible way to get this to work, I'll open up a new issue and leave it open.

Okay, this is weird.

Here's how vim-ruby currently indents this case:

one.
  three do
  "four"
end

# but,

one.
  two.
  three do
    "four"
  end

The indenting of the block depends on how many continued lines there are.

The good news is, I can "fix" this by just deleting some code. I've done that in a7f0ea412b7680d90122976f3361ea248d4dc5b6.

The bad news is, I'm not sure what this code was supposed to handle :). @tpope, could you take a look at the commit and let me know if you can think of why this code would exist? I'd also really appreciate some ideas on what the "right" indent would even be in this case.

@kballenegger, I've created a branch called fix-blocks-in-dot-continuations that you could try. Although I'm not sure you'll like the way I've implemented it. Your example now indents like this:

a = more.
  traditionally.
  style do
  pipeline
end.is.styled.
  correctly

but.does.not.reset.correctly.after

I find this perfectly sensible, although understandably ugly. Personally, I'd go with:

a = more.
  traditionally.
  style { pipeline }.is.styled.
  correctly

but.does.not.reset.correctly.after

But I guess you might want to have more contents in the block... I'll think about whether it makes sense to handle dot-continuations in the same "hanging" way as =-continuations are handled. Not sure "how" I'd do that, but that's a different topic.

I think the case that's a little ambiguous is your "real" example above:

I would venture to say that there are probably two elegant formattings:

[1, 2, 3].select do |i|
  i.odd?
end.map do |i|
  i * i
end

and

[1, 2, 3]
  .select do |i|
    i.odd?
  end
  .map do |i|
    i * i
  end

That said, I'd probably write this one like this:

[1, 2, 3]               \
  .select {|i| i.odd? } \
  .map {|i| i * i }

Yeah, both the first and the third example indent correctly, and those are the ones I'd prefer. It's the second example that's the tricky one.

I agree that this looks weird:

[1, 2, 3].select do |i|
  i.odd?
end.
  map do |i|
    i * i
  end

Then again, this doesn't:

beginning_line_was_just.
  too_long do
  "four"
end

Or at least you can definitely see that it's the first line that was too long and the contents are indented according to the first line. For longer blocks of code in particular this might make a lot more sense. If you end up breaking the first line of the block simply because it ended up too long, it makes no sense to reindent the entirety of the contents differently. It would make sense in the first example, simply because they're small snippets that form a pipeline of small operations and the first line has a different role than the rest:

subject
  .method_call do |i|
    i.odd?
  end
  .method_call do |i|
    i * i
  end

But then maybe in that case it makes a lot more sense to just write one-liners with curly braces.

I don't know. It's an edge case, and, at least to me, both formattings make sense in different situations. I don't think a whole lot of people care about this anyway, so I'll try to change it so it indents like you prefer it, indented according to the do line (written in my preferred style regarding the dots):

[1, 2, 3].
  select do |i|
    i.odd?
  end.
  map do |i|
    i * i
  end

Not sure how long it'll take, as you might have noticed, it takes me a while to get around to working on vim-ruby :). But I'll see what I can do.