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

Ruby: Ternary and case support #23

Merged
merged 21 commits into from
Feb 2, 2014
Merged

Conversation

LFDM
Copy link
Contributor

@LFDM LFDM commented Feb 1, 2014

Closes #22

  • Adds splitjoin functions to handle Ruby's ternary operator
    An argument can be made to use the other tense one-line statement for Ruby's if-else construct, but that looks more awkward to me than the good old ternary operator (if condition then x else y end - having an end lingering around on the line makes this look pretty weird imo)
    It also handles variable assignments, adding parentheses to the ternary itself, as in: x = (condition ? true : false). Specs are included.
  • Adds splitjoin function to hande Ruby's case switches
    There are two triggers: You can invoke splitjoin on single when lines, or invoke on the case line itself, which calls the when functions on all included when-then statements, including the else case. Confer to the spec file to see what it exactly does (and what it doesn't!)
    You will notice that one spec example is commented out: For tense case switches that only include when-then one-liners, the alignment tools are invoked. This is at the moment however impossible to test. It probably makes no sense to add an alignment tool to the spec dependencies, and I don't think that this experimental spec platform can handle stubbing of the Tabular or Align plugin. I left the spec in for referencing purposes. Your decision how you want to handle this of course!

If you want I can add more descriptive comments to the code - I haven't done yet because the rest of the code is also not drowned in comments. Code is also relatively simple to follow, but as I said, would be no problem to add more!

Thx for splitjoin (and switch, while we're at it 👍)!

if end_line_no > 0
let lines = sj#GetLines(if_line_no, end_line_no)

let if_line = lines[0]
let end_line = lines[-1]
echo lines
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is a forgotten debug line? Could you remove it? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@AndrewRadev
Copy link
Owner

Don't worry, I don't have any specific problem with ternary clauses. I'm not sure about the brackets (I use brackets for ternary clauses myself, but it's likely that many people don't), but let's leave them like this and if someone complains, I'll make an option. I'm also okay with the amount of code comments.

I have a bunch of comments on various things in the code, but it's mostly quibbles about whitespace or naming. I hope that it won't be too hard to fix them, though if you disagree on any, let me know.

I would also appreciate it if you documented the logic in the doc files as well, at the end of this: https://github.com/AndrewRadev/splitjoin.vim/blob/master/doc/splitjoin.txt#L171-236. A single example for the ternary clause and for the case statement will suffice, no need to go into detail -- users can try it out for themselves ;).

Other than that, great work! If you could make the changes I pointed out, I'll merge immediately, though don't hesitate to let me know if you disagree with me on any of these points.

@LFDM
Copy link
Contributor Author

LFDM commented Feb 2, 2014

Many thanks for your suggestions - much appreciated! I think I fixed all issues now.

You've hit a nerve with your comments on the parens around ternaries. I usually use them for the sake of clarity as well, especially for something like var = (opt == :sym ? 1 : 2) because var = opt == looks a little off to me, even if Ruby handles it just fine.
I also have a switch defined for ternaries (changing the true and false path), the ending ) makes the regex a lot more complex - in fact, I failed to implement it. The ending ) is of course optional in a switch regex, but the non-greedy * operations seem a little weird in vim's regex, I gave up on it as I didn't want to waste time on it... A nested dict might do the trick more easily, but well, that's not an issue of this PR!

Thanks again for helping out!

AndrewRadev added a commit that referenced this pull request Feb 2, 2014
Ruby: Ternary and case support
@AndrewRadev AndrewRadev merged commit c03536b into AndrewRadev:master Feb 2, 2014
@AndrewRadev
Copy link
Owner

Thanks again for your work 👍

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.

Ternary support for ruby
2 participants