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
[New Command] Expand PPX nodes #1745
base: master
Are you sure you want to change the base?
Conversation
src/kernel/mbrowse.ml
Outdated
| (_,Structure str) -> | ||
of_structure_items (List.filter ~f:(fun (_,x) -> | ||
check_node pos (Structure_item (x, str.str_final_env)) | ||
) (List.map ~f:(fun x -> (str.str_final_env, x)) str.str_items)) | ||
| (_,Signature str) -> | ||
of_signature_items (List.filter ~f:(fun (_,x) -> | ||
check_node pos (Signature_item (x, str.sig_final_env)) | ||
) (List.map ~f:(fun x -> (str.sig_final_env, x)) str.sig_items)) |
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 guess that we can simplify get_children
using filter_map
or moving some logic in of_(signature|structure)_items
. WDYT?
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.
This looks very nice and is a very useful feature. Thanks a lot, @PizieDust! two general comments:
As a suggestion for an addition to the tests: It would be interesting to test what happens when you use the feature on a deriver that's not registered via dune
. Or when you use it on a deriver that triggers some other error, e.g. you call it with a flag
, when it doesn't support flags, or it expects some value to be in scope that's not in scope. In those cases, do the errors appear as "derived code" or not?
And as a nit-pick: merlin-lib
doesn't know about hover, so the title of the PR is a little misleading. The part of your work about hover is the related work you've done on ocaml-lsp.
closes #1744
This implementation has support for: