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

It starts from the wrong split #48

Open
firedev opened this issue Oct 13, 2014 · 5 comments
Open

It starts from the wrong split #48

firedev opened this issue Oct 13, 2014 · 5 comments

Comments

@firedev
Copy link

firedev commented Oct 13, 2014

Given:

    facilities = facilities.flatten.map { |item| item.is_a?(Facility) ? item.id : item.to_i }

After pressing gS I expect it would start from the outside and convert it to:

    facilities = facilities.flatten.map do |item|
      item.is_a?(Facility) ? item.id : item.to_i
    end

Instead i get this, which is wrong and not even ruby:

    facilities = if facilities.flatten.map { |item| item.is_a?Facility
                                             item.id
                 else
                   item.to_i }
                 end
@AndrewRadev
Copy link
Owner

I've pushed a fix that should improve the situation. As long as you split from within the curly braces, it should work.

This is still not optimal, since, if you try to split while on map, it still splits the ternary clause into an if-else statement. Ideally, it shouldn't do anything (well, ideally it would figure out there's a block following and split that, but I can't figure out how to pull this off reliably).

I'll see what I can do about fixing the ternary clause splitting.

@firedev
Copy link
Author

firedev commented Oct 14, 2014

It splits now correctly if I put cursor on { after map, thanks. And joins it back beautifully. However if the cursor is at the beginning of the line everything explodes.

Is there a way to detect possible splits from the outside in? So it checks for a first { or ( and converts it to do/end and (<cr>/<cr>).

@AndrewRadev
Copy link
Owner

It's not that easy, I'm afraid. If nothing else, this would be possible in some cases, but not in others, which means the different splittings would work in different positions. Right now, the rule of thumb is "be inside the structure you're splitting". Anything else is pretty complicated.

To give you an example:

method_call("Some long string") { |block| block.call }

If you're in the block, this will split into

method_call("Some long string") do |block|
  block.call
end

If you're in the string:

method_call(<<-EOF) { |block| block.call }
Some long string
EOF

However, if I were to implement "check for a curly brace after the cursor as well", the string would never be splittable in that position.

@AndrewRadev
Copy link
Owner

But, as I said, it should not blow up like this. It should simply limit its splitting of ternary clauses. I'll see what I can do about that.

@firedev
Copy link
Author

firedev commented Oct 26, 2014

After thinking about it, I guess you are right and it would be best if it just split at cursor position to avoid ambiguity and to have less "magic".

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

No branches or pull requests

2 participants