ocaml / merlin

Context sensitive completion for OCaml in Vim and Emacs
https://ocaml.github.io/merlin/
MIT License
1.58k stars 233 forks source link

[emacs] feature: have merlin-show-type-at-point pick enclosing expressions, not just identifiers #6

Closed gasche closed 11 years ago

gasche commented 11 years ago

caml-mode's caml-types-show-type (code here) picks the smallest expression around the point, rather than only supporting identifiers, and uses an overlay to make it visually clear of which subexpression the type is being displayed. In the example:

let test = (List.length [])

I expect C-c C-t to work when called on the leftmost parenthesis (highlight the expression and return "int") and on either braces of the empty list (highlight [] and return '_a list).

The larger point is that I expect merlin's C-c C-t to have the same feature set than caml-mode's C-c C-t command, because I don't want as an user to have to remember which one to use depending on whether I'm doing incremental checking or reading an -annot-compiled source file. I understand that some features may be painful to implement just from the typer output (eg. handling of weak polymoprhic variables as demonstrated with the empty list here), but at least the best-subexpression logic could be reused.

asmanur commented 11 years ago

caml-mode's caml-types-show-type (code here) picks the smallest expression around the point, rather than only supporting identifiers, and uses an overlay to make it visually clear of which subexpression the type is being displayed. In the example:

let test = (List.length [])

I expect C-c C-t to work when called on the leftmost parenthesis (highlight the expression and return "int") and on either braces of the empty list (highlight [] and return '_a list).

The larger point is that I expect merlin's C-c C-t to have the same feature set than caml-mode's C-c C-t command, because I don't want as an user to have to remember which one to use depending on whether I'm doing incremental checking or reading an -annot-compiled source file. I understand that some features may be painful to implement just from the typer output (eg. handling of weak polymoprhic variables as demonstrated with the empty list here), but at least the best-subexpression logic could be reused. This is indeed not so easy to implement, perhaps the binary has a command for that ?

let-def commented 11 years ago

Ok, I take care of solving the issue. This in fact has to be handled by the binary, but nothing is provided in this direction at this time.

trefis commented 11 years ago

To clarify : the feature you requested is already implemented gasche (in vim, we access it with :TypeCursor ) I tried it on your examples, it worked as expected. As well as on polymorphic variants as described on the issue #5 . We discussed it with asmanur earlier today and I'm guessing he will patch the emacs mode soon.

The only thing missing from what you described is : the position of the matched expression is not returned, so at the moment we don't have a nice highlight showing what was matched. The last patch from def makes that possible (although it could probably have been done without adding a new command), so you should see everything you dreamed of in emacs as soon as asmanur gets back (or you could do it yourself, if elisp doesn't scare you).

let-def commented 11 years ago

I modified the "type" "at" command to include location of expression. vim mode has been updated, the emacs mode doesn't implement this command at the moment.

asmanur commented 11 years ago

I can modify the current "type" "at" command, and I think I will do it anyway… But since it will break emacs and vim modes, I will wait for asmanur to be here and patch the elisp interface This should not actually break the emacs mode :-)

Anyway we discussed on IRC with thomas on how this feature should be implemented. There is now three ways to get the type of the expression under point:

  1. use type expression at where is some that is located by the editor (so using heuristics)
  2. use type at which prints the type of the AST node under point
  3. use type expression to type globally.

Emacs tries first 1. and then 3. I can add support for 2. but then which one should I try first ? 1. or 2. ? Thomas told me that 2. might be less accurate than 1. (provided the editor knows how to find the expression under point).

gasche commented 11 years ago

I think it's best if code logic is located in merlin itself rather than in the editor modes. I would attempt to have an interface such that merlin itself can do the work of "locating the expression" if needed (I understand that may require the emacs-mode to send more input to merlin, as in the other discussion of detecting the end of definition), and have the editor modes do the dumb thing of "what's the type here" only.

When is 2. less accurate than 1.? Remark that 3. can be wrong as well:

let x = int
let test =
  let wrong = parse error \ here in
  let x = true in
  (* type that: *) [x]

My hunch would be to use 2. by default (extended to subsume 1. if needed), and fallback on 3., but with an interface feedback that would let the user know that 3. answer is actually different in kind (eg. "Local type unavailable, but the toplevel type is blah"). It's better to be sometimes a bit wrong than never say anything, but you also want the user to trust your input so it's better to be explicitly about how sure you are it's correct.

asmanur commented 11 years ago

I think it's best if code logic is located in merlin itself rather than in the editor modes. I would attempt to have an interface such that merlin itself can do the work of "locating the expression" if needed (I understand that may require the emacs-mode to send more input to merlin, as in the other discussion of detecting the end of definition), and have the editor modes do the dumb thing of "what's the type here" only. It makes sense. I have implemented a version of it in a new branch emacs-dev. I have kept for now the order 1-2-3, so to see it it's better to call the function directly by issuing:

M-: (merlin-type-of-expression-node)

Here it what it looks like: http://kiwi.iuwt.fr/~asmanur/emacs-type.png

gasche commented 11 years ago

That's great, thanks. There is a bit of a naming mismatch as I want to use that to type-check patterns and module-expressions as well (might be merlin-type-of-node would be better for this reason).

Off-topic: do current emacs mode support a "select-node-under-point" command? If not, merlin may provide such a feature using the same (future) logic as option 2.

asmanur commented 11 years ago

That's great, thanks. There is a bit of a naming mismatch as I want to use that to type-check patterns and module-expressions as well (might be merlin-type-of-node would be better for this reason).

Off-topic: do current emacs mode support a "select-node-under-point" command? If not, merlin may provide such a feature using the same (future) logic as option 2.

what does select mean in this context ?

gasche commented 11 years ago

"marking" (making the expression as a region) in emacs parlance, sorry. In fact this remark stems from the fact that I was thinking it could be useful to mark the node under point by default when you call C-c C-t. This may be what you're already doing (instead of just a highlight as caml-mode) -- you haven't pushed your changes yet. It may be enough to have a single command "mark expression and give me type feedback", but people may also request feedback while they already have a region they want to keep, and I would want a marking command to also work when the code before is not type-valid but can be parsed reasonably.

let-def commented 11 years ago

I agree that the code logic should be in merlin and not in the editor, but right now the better is to work around merlin's limitations in the mode.

The vim mode tries 1. then fallback to 2. and prefix "(approx)" to the result.

gasche commented 11 years ago

How do you see the required changes in merlin? If wouldn't mind having a try at implementing something for that (... maybe, if time allows). I'd be glad to help with the code, but not the Elisp code, so I haven't had any opportunity to hack yet.

let-def commented 11 years ago

The expected behavior, I think, would be to detect and give type for patterns and labels (to match "type expression") and to be more robust against parsing and typing errors (I may merge that soon).

Otherwise, I think it is ok that the editor can still fallback to explicitly giving an expression.

I am not sure "3." is useful, except if a definition has been erroneously shadowed in the local environment : if there is an environment available near cursor (would it be in the previous definition), the "2." will find it.

asmanur commented 11 years ago

I have implemented most of what you mentioned. Please try it and open new issues if you still feel the behavior is not quite what you were expecting.

There is no "mark expression" things yet but the heuristics used by merlin would not make this very useful I would say (but I'm not sure what would be the use anyway so...)