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

Highlight visible kind application, fixes to visible type application #135

Merged
merged 2 commits into from
May 2, 2020

Conversation

sheaf
Copy link
Collaborator

@sheaf sheaf commented Apr 30, 2020

Type application is now detected by: space, then @, then an expression of the following three forms:

  • between (, )
  • between [, ]
  • a sequence of allowed characters as defined by [\p{Ll}_\p{Lu}\p{Lt}\p{Nd}'']

I added some tricky situations to the test case to check that it's behaving as expected, but please add more cases if you can spot other issues/potential issues.

Note that we're doing better than GitHub on this one! 😄

Fixes test case T0073.

@@ -573,6 +547,35 @@ repository:
| } # A block closed? Maybe this should also include `;`, because non-indentation based `do`
)
| ^(?!\1\s+\S|\s*$) # at least one, no-further indented, non-whitespace character. I.e. a same-level declaration/implementation
type_application:
patterns:
- begin: '(?<=\s)(@)(\()'
Copy link
Owner

Choose a reason for hiding this comment

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

I am not sure about the look-behind here. As your original issue (#73) quoted, the @ may not be preceeded by any end-of-identifier character, which is why I just copied the legal identifier character class regex. It may be that the majority there overlaps with the inversion of \s, but at least in cases where you have (my expr)@type it should probably still parse as a type application, what do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

I realize this is a very niche case though 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. It seems a bit subtle indeed, as actually

f = (id)@([Int])

is not parsed by GHC, whereas

f = [1,2,3]@[Int]
g = A{f=1}@Bool

are both parsed as type applications. I'm not sure exactly then which are the relevant characters, do you have a suggestion for a regex? Like [\s\}\]] or something.

name: meta.type-application.haskell
patterns:
- include: '#type_signature'
- begin: '(?<=\s)(@)\s*(\[)'
Copy link
Owner

Choose a reason for hiding this comment

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

now that I think about it ... do you think we could somehow only trigger on the @ (plus look behind) and then dispatch into another set of begin-end rules for [ ], ( ) and ident expressions? The termination condition for the outer multiline match would be the concatenation of the inner end conditions, perhaps as a look-behind. This way we don't have to duplicate the complicated start condition so many times? I think it could actually work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I haven't been able to figure this one out. Feel free to give it a go, otherwise I think I'll just use the current solution.

@sheaf sheaf marked this pull request as draft May 1, 2020 18:48
@sheaf sheaf marked this pull request as ready for review May 1, 2020 19:14
@sheaf
Copy link
Collaborator Author

sheaf commented May 1, 2020

I used the lookbehind (?<=[\s,;\[\]{}"]) for invalid end-of-identifier characters. Parentheses are not included because you could have something like

(:+:)@(A B C)

which is valid record at syntax.

@JustusAdam I hope this is OK now. I don't know how to avoid the triple lookbehind for the visible type application, let me know if you have a solution.

@sheaf sheaf marked this pull request as draft May 1, 2020 23:13
@sheaf
Copy link
Collaborator Author

sheaf commented May 1, 2020

I just realised we are also allowed the following:

f @'(a,b) 
f @'[a,b,c]

I'll add those to this PR soon.

Edit: Done (+rebased).

@sheaf sheaf marked this pull request as ready for review May 2, 2020 01:32
@sheaf sheaf merged commit a63c51e into JustusAdam:master May 2, 2020
@sheaf sheaf deleted the vta branch May 15, 2020 23:49
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.

2 participants