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

Improve multiline formatting #7

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

i-tsvetkov
Copy link
Contributor

  • Improve the indentation of multiline code by taking into account the size of the opening ERB tag.
  • Improve the limiting of line length by adjusting for the indentation and size of the opening ERB tag.
  • Upgrade syntax_tree to the latest version.

@elia
Copy link
Member

elia commented Jan 11, 2023

@i-tsvetkov sorry for the delay, this fell completely off the radar 📡

First of all thanks for taking the time to dig into the code, that was brave! 😄

I'm a bit conflicted about the change, my initial drive for ruby code was for having it indented relatively to the beginning of the ERB tags (like it is currently) and for parentheses-less method calls the workaround was to add parens.

In my view the ideal situation would align to the method call for parens-less calls and as explained above when parens are present.

Thoughts?

@i-tsvetkov
Copy link
Contributor Author

Hi @elia, I'm glad you found the time to look into this PR.

I saw that you have extracted the upgrade of syntax_tree into #10,
you should know that there was an issue with newer versions regarding the transformation of short blocks like this one

[1, 2, 3].each do |i|
  puts i
end

into single-line form

[1, 2, 3].each { |i| puts i }

That issue was resolved by this commit.

Regarding the question about the indentation - I think that multi-line code should be indented relative to the start of the ERB tag content (right after the <% ) instead of the start of the ERB tag itself (just before the <% ) that way we responsibility and complexity of formatting ruby code will be handled by syntax_tree.

Here is an example with an if-else statement:

# relative to the start of the ERB tag content
# the if-else-end is vertically aligned
<%= if rand < 0.5
      puts 1
    else
      puts 2
    end %>

# relative to the start of the ERB tag itself
# the if-else-end is not vertically aligned
<%= if rand < 0.5
  puts 1
else
  puts 2
end %>

What do you think?

@i-tsvetkov
Copy link
Contributor Author

@elia Any update on this PR?

@mtomov
Copy link

mtomov commented May 31, 2023

Hello,

Thanks for erb-formatter!

I was wondering where you'd like to take this PR?

I find the proposed style to be nicer than current. In fact, when trying out erb-formatter, the lack of indentation with multi line arguments surprised me. Wondered what the reasoning for that choice was as my preference would be the same as this PR proposes.

Thank you!

@mtomov
Copy link

mtomov commented Jul 7, 2023

Hey @elia

We've manually updated the gem locally with this PR changes, and has been working great so far. Any chance you could revisit this?

@elia
Copy link
Member

elia commented Jul 7, 2023

@mtomov on it, thanks for the ping, I'm pondering options 🙏

@elia elia force-pushed the improve-multiline-formatting branch from 314f847 to 72d613e Compare July 7, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants