Indent Conditional Assignment

[rocketbop]

Looked in issues to see if this was discussed before, but haven't seen any.

Conditional blocks go against the Ruby style guide in terms of space efficiency at the moment.

https://github.com/rubocop-hq/ruby-style-guide#indent-conditional-assignment

Rufo changes

result =
  if some_cond
    calc_something
  else
    calc_something_else
  end

into

result = if some_cond
  calc_something
else
  calc_something_else
end

A lot of Ruby coders use narrow char widths and this seems to go against that. Is it a conscious decision or omission? If it's an omission I'll happily make a PR.

[gingermusketeer]

@paublyrne with the latest version of rufo the code becomes:

result = if some_cond
    calc_something
  else
    calc_something_else
  end

This was chosen recently, see #216 and linked issue comment #90 (comment), but definitely something that can be refined. The Ruby style does not mention the style above so I think it should not be considered authoritative.

Looking through the following repos:

Homebrew/brew
Netflix/fast_jsonapi
rails/rails
rails/rails
ruby/ruby
ruby/spec

I found the following:
if is on a separate line to assignment operator: 73
if is on the same line as the assignment operator: 452

I used ripgrep to do this with rg "=\n\s*if" --type ruby -U --stats and rg "=\s*if" --type ruby --stats respectively.

Let me know your thoughts on the above. Also are you suggesting that we change to your preferred style for everything or only for long lines?

[rocketbop]

Oh yes I see that the rufo way is something in between the examples in the Ruby Style guide.

Although my preference is for their final example

result =
  if some_cond
    calc_something
  else
    calc_something_else
  end

In the comment you link this would be Option 1.

But it's personal preference, I could live with the current approach. Would prefer that at least for long lines it switches to the above.

I do note that Option 1 was the only option with no cons though 😉

[gingermusketeer]

@paublyrne if you want to tackle the long line case then I would happily accept a PR. We don't have line length logic in the code base at the moment so I think it will be non trivial.

We do have a branch which is working on a new formatter that would be able to handle things like this however.

At the moment I am focused on addressing issues that would block usage. For example I would like to get the HEREDOC problems resolved.

Let me know if you are interested in tackling this otherwise I think it is best to close this issue for now.

[rocketbop]

Sounds good. I can take a look at the length issue this week.

[caifara]

First of all: thank you for this encredible gem!

I saw several people in #90 trying to move rufo to some new form. Just adding my 2 cents here for what it's worth. I find the form where the conditional stays completely to the right of the assignment to best communicate the intent of the code.
I wonder if the amount of users regretting the change wouldn't be larger than the one requesting the change as the ruby style guide and rubocop also recommend that form.

I couldn't find an example of the current formatting in the wild, but to be fair, I didn't do an exhaustive search.

Rspec uses the "conditional on the right of the assignment" form as suggested above. Rails uses the "bad" format according to the ruby style guide.

Anyway, rubocop is pretty annoyed by the change introduced by #216 :-).

[rocketbop]

I'm happy to close this if it's contentious.

[aaronjensen]

Just found this since the current method conflicts with Rubocop, which does not appear to have this as an option, but they do have 3 others: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Layout/EndAlignment

I can disable the rubocop rule, but it might be nice for them to somewhat get along. Thanks!

[gingermusketeer]

@caifara @aaronjensen I would not recommend using both Rufo and Rubocop for styling. Rufo includes rubocop configuration that you can use to disable formatting rules.

Best to let one tool decide rather than having two fighting each other. This is how it is handled in other communities such as Rust and Go which both have seperate tools for linting and formatting.

I am going to close this for now as I think the current format is better than the other options due to smaller code diffs while still communicating intent.

[aaronjensen]

@gingermusketeer could you point me to this configuration and associated docs if available, please? Is it https://github.com/xinminlabs/rubocop-config-rufo ?

[gingermusketeer]

@aaronjensen you can find it here: https://github.com/ruby-formatter/rufo/blob/master/.rubocop.yml

[aaronjensen]

Ah, I was thinking it would be a configuration that I could extend in my configuration. This looks like it also disables metrics, which seems likely orthogonal to ruby formatting, but the other bits are easy enough to copy into our config. Thank you.

[gingermusketeer]

@aaronjensen I don’t recall off the top of my head why it disables metrics but if you would like to make it reusable then PRs are definitely welcome