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] Add lambda operator to the list of builtin functions #2411

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

joseramonc
Copy link
Contributor

This is mostly to fix the ligature issue, since -> is currently tokenizing as arithmetic and comparision

Before:
image
image

After (ligature works):
image

@jwortmann
Copy link
Contributor

I assume -> here in Ruby has the same meaning as in Java or => in JavaScript and C#. I think the scope should not be support.function here, because that's meant for built-in function names like e.g. println instead. Here are some alternatives which are currently used in this repository:

I think the scope name used in D is the most appropriate here and up to date with the scope naming guidelines.

@joseramonc
Copy link
Contributor Author

Thanks for your feedback, I just changed the scope to match D, however, I'm not sure why tests failed (seems unrelated to this) or is there anything else I need to do regarding adding this lambda operator?

@michaelblyons
Copy link
Collaborator

I think the tests are not your fault. See #2328

@wbond
Copy link
Member

wbond commented Jul 17, 2020

Yeah, I'm working on getting the CI test runner fixed

@wbond
Copy link
Member

wbond commented Jul 20, 2020

If you rebase on master, the CI should be working now.

@joseramonc joseramonc force-pushed the add-lambda-operator branch from e2037af to 833f18e Compare July 21, 2020 00:39
@joseramonc
Copy link
Contributor Author

Awesome, just rebased and it's passing now. Thanks!

@@ -1128,6 +1128,8 @@ class MyClass
exit! 2
#^^^^ support.function.builtin

->
#^^ meta.function.ruby storage.type.function.ruby keyword.declaration.function.lambda.ruby
Copy link
Member

Choose a reason for hiding this comment

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

This is a good first step, but perhaps provide a slightly more detailed example also? I'd recommend perhaps some parameters and a simple function body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I've pushed a better example for the ->

@wbond wbond merged commit 965d695 into sublimehq:master Jul 22, 2020
@wbond
Copy link
Member

wbond commented Jul 22, 2020

Thanks!

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.

4 participants