-
Notifications
You must be signed in to change notification settings - Fork 16
Add type hovers for jkinds #200
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
Conversation
dkalinichenko-js
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would I test this in an editor?
|
I also expect that we'll want more tests once we have jkind abbreviations. |
I can walk you through this in-person. It'd be difficult to do this in vscode because I believe an LSP change will also be necessary. But I believe that emacs just calls this type-enclosing command directly, so that would be a matter of building Merlin locally and then pointing emacs at it. |
dkalinichenko-js
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I'm good with approving this based on merlin tests + the fact that you tested this then.
| $ hover 10 18 1 | ||
| val f : ('a : immediate). 'a -> 'a | ||
| ^ | ||
| "immediate)" : "value mod global many stateless immutable external_ non_float" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it print the closing bracket here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't invesitgated this much, but I suspect that it's a bug in the parser where the wrong location is being recorded. There's some other tests in this file where an extra following character is printed (but in those other cases, it's a space rather than a paren).
This PR makes Merlin support hovering a jkind abbreviation to see the expanded form of the abbreviation.