-
Notifications
You must be signed in to change notification settings - Fork 90
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
Ternary support for ruby #22
Comments
Ah, ok there are actually specs now, I should have looked beforehand - rspec like, cool! |
Another update: Cases can be splitjoined now as well. |
This looks pretty good. The reason I haven't worked on ternary clauses is because I don't use them much, I guess. They're also quite easy to put in any other expression, so hard to detect. But the simple case that you handle (assignment) seems like a perfectly reasonable use case. The If you provide a pull request, I'm going to request some minor (I think) changes to naming and comments. In general, you've kept very nicely to my style of coding, so I don't have many complaints. I noticed there's one commented out spec, but I can take a more serious look at it after the PR. Thank you for your work, specs included :). |
I understand that there are people who don't like the ternary but I am not one of them. It's was almost everything else in Ruby: if you use your tools sanely, you're all good. Nesting ternaries f.e. is a no-go. The rest I'll comment in the PR that will pop up in a few minutes! |
I'm going to reopen this issue, because I've run into a bit of a problem. Issue #48 illustrates it. The problem is that the ternary pattern is very broad and takes effect even when the cursor is not on it, and even when it doesn't really make sense to split. The example there is indicative: facilities = if facilities.flatten.map { |item| item.is_a?Facility
item.id
else
item.to_i }
end I think that the entire start of the line is captured as the beginning item in the ternary clause. The brackets also get captured in a weird way. I tried to scope the pattern a bit, but right now, I can't see a way to do this reliably. What should happen is that the ternary should only be split when the cursor is on it, but the start of the ternary could be basically anything :/. Not sure what I can do about it, short of removing the ternary support altogether (for splitting at least). Any ideas? |
Sorry, I kinda missed the notification for that - was just thinking to do something html related and ran across this. Will investigate at some point when I am warmed up with the code (and vimscript) in general again ;) |
The safest thing is probably to split the ternary only in one specific case: When we are directly on the Or even better: |
Being directly on the foo = (bar == baz) ? "bar" : "baz" Also, I'm still not completely sure how detection would work. I think the problem here is that, even if you find the facilities = facilities.flatten.map { |item| item.is_a?(Facility) ? item.id : item.to_i } I guess we could have a list of operators that "stop" the search, like |
Hey @AndrewRadev, would you be interesting in adding ternary support for Ruby to the built in feature set?
Here's what it does: https://gist.github.com/LFDM/8721142
And here's how its implemented: https://github.com/LFDM/splitjoin.vim/blob/ternary_support/autoload/sj/ruby.vim#L53-160
A slight change to the IfClause functions was needed, but all for the better: They now return false when there is an else case present - tests of course still green.
I'll add a PR and tests if you're interested.
The text was updated successfully, but these errors were encountered: