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

Recognizing keyword argument in f(x; kwd) #114

Open
fredcallaway opened this issue Nov 3, 2021 · 11 comments
Open

Recognizing keyword argument in f(x; kwd) #114

fredcallaway opened this issue Nov 3, 2021 · 11 comments

Comments

@fredcallaway
Copy link

It would be great to tag kwd in the f(x; kwd) be tagged with variable.parameter.julia. I thought it would be pretty simple to recognize the first semicolon and then push a new context which is identical to function-parameters, but where anything tagged as an argument is also tagged as a parameter. But there's some fancy branching stuff going on here that I couldn't grok so I couldn't make it work myself.

Here's a test (which fails)

  bar(x; y, kws...)
#        ^ meta.function-call.arguments.julia variable.parameter.julia

I'm not sure whether kws should be a parameter as well. It is acting as a keyword argument, but it's not actually the name of a parameter. I think it's technically more correct to not tag it, but it would be more practically useful to tag it because from a user perspective, we really just want to know whether the variables are getting passed as keyword arguments or regular arguments.

@jwortmann
Copy link

jwortmann commented Nov 9, 2021

Maybe this works? I don't know if I missed the branching stuff.

--- Installed Packages/Julia/Julia.sublime-syntax
+++ Packages/Julia/Julia.sublime-syntax
@@ -1034,15 +1034,32 @@
             pop: true
           - include: main
 
+  function-call-keyword-arguments-only:
+    - meta_content_scope: meta.function-call.arguments.julia
+    - match: \)
+      scope: punctuation.section.arguments.end.julia
+      set: after-expression
+    - match: \,
+      scope: punctuation.separator.comma.julia
+    - include: function-call-keyword-arguments
+    - match: '{{symb_id}}(?=\s*(?:\,|\)|\.{3}))'
+      scope: variable.parameter.julia
+      push: after-expression
+    - include: main
+
   function-call-arguments-group:
     - match: \(
       scope: punctuation.section.arguments.begin.julia
       set:
-        - meta_scope: meta.function-call.julia
-        - meta_content_scope: meta.function-call.arguments.julia
+        - meta_scope: meta.function-call.arguments.julia
         - match: \)
           scope: punctuation.section.arguments.end.julia
           set: after-expression
+        - match: \,
+          scope: punctuation.separator.comma.julia
+        - match: \;
+          scope: punctuation.separator.semicolon.julia
+          set: function-call-keyword-arguments-only
         - include: function-call-keyword-arguments
         - include: main

I've also added punctuation scopes for , and ; and fixed the meta scopes (I think they are not supposed to stack). The adjusted meta scopes will break a lot of syntax tests though.

plot(x, y; width)
#          ^^^^^ variable.parameter.julia

plot(x, y, width=width)
#          ^^^^^ variable.parameter.julia
#                ^^^^^ - variable.parameter

plot(x, y; width=width)
#          ^^^^^ variable.parameter.julia
#                ^^^^^ - variable.parameter

plot(x, y; :width => width)
#          ^^^^^^ constant.other.symbol.julia

plot(x, y; width=options.width)
#          ^^^^^ variable.parameter.julia

plot(x, y; options.width)
#          ^^^^^^^^^^^^^ - variable.parameter

plot(x, y; width, kws...)
#   ^^^^^^^^^^^^^^^^^^^^^ meta.function-call.arguments.julia - meta.function-call meta.function-call
#          ^^^^^ variable.parameter.julia
#                 ^^^ variable.parameter.julia
#                    ^^^ keyword.operator.splat.julia
#     ^ punctuation.separator.comma.julia
#        ^ punctuation.separator.semicolon.julia
#               ^ punctuation.separator.comma.julia

@fredcallaway
Copy link
Author

Forgive my ignorance, but is there a good way to test these changes? I tried using git apply but it says this is a "corrupt patch"

@randy3k
Copy link
Collaborator

randy3k commented Nov 9, 2021

+        - match: \;
+          scope: punctuation.separator.semicolon.julia
+          set: function-call-keyword-arguments-only

I don't think it is correct as it pops the current context out. We should only pop it after \).

@jwortmann
Copy link

If the Julia package is installed via Package Control, then it can't work because the syntax is within a zip file in the Installed Packages folder. If you have the package git cloned, then the diff above probably has the wrong paths or it doesn't work because there are no commit hashes? I didn't create the diff via git diff, because I don't have the Julia package git cloned.

An easy way (and what I did) was to create a folder Julia in the Packages directory, create an override for the syntax file by copying it there, and then just copy the file lines from the diff by hand.


I don't think it is correct as it pops the current context out. We should only pop it after ).

Not sure why that would be a problem. The meta scope is applied in the new context too, and this new context also pops after \).

@randy3k
Copy link
Collaborator

randy3k commented Nov 9, 2021

Then how's \) be matched?

@jwortmann
Copy link

By the exact same rule as in function-call-arguments-group?

@randy3k
Copy link
Collaborator

randy3k commented Nov 9, 2021

By the exact same rule as in function-call-arguments-group?

But in

+        - match: \;
+          scope: punctuation.separator.semicolon.julia
+          set: function-call-keyword-arguments-only

The context will be popped, set it sees a semicolon. There is no way that the \) to be matched.

@randy3k
Copy link
Collaborator

randy3k commented Nov 9, 2021

My bad, I didn't see that there is a match for \) in function-call-keyword-arguments-only.

@randy3k
Copy link
Collaborator

randy3k commented Nov 9, 2021

But I think a better way is to push and then pop in function-call-keyword-arguments-only when matching (?=\)).
(This may be also true for many of the existing logic. At some point, I'll need to revise the logic in the syntax.)

@jwortmann
Copy link

But I think a better way is to push and then pop in function-call-keyword-arguments-only when matching (?=)).
(This may be also true for many of the existing logic. At some point, I'll need to revise the logic in the syntax.)

Yes this alternative would work too and maybe is a better way. In that case, the new context doesn't need the meta scope.

@fredcallaway
Copy link
Author

fredcallaway commented Nov 19, 2021

@jwortmann's seems to work in practice, but it breaks a lot of tests, I think all because [meta.function-call.julia] does not match scope [meta.function-call.arguments.julia]

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

3 participants