ocaml / merlin

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

Refinement in the presence of optional arguments #1800

Closed xvw closed 4 months ago

xvw commented 4 months ago

~I don't know whether the fixed system might break some of the parameters generated by PPX. (Perhaps if there is a ghost on the parent expression, you can check that it is the None constructor?).~

After a conversation with @pitag-ha , we came to the conclusion that the introduction of ghost location in this context (which, in PPX terms, should be reserved for derivers) is a problem of semantics ‘on the PPX side’, so that this fix, which is based on ghost location, is not problematic (for PPX).

Fix #1770

xvw commented 4 months ago

(Comment referencing the issue was added)

liam923 commented 4 months ago

Thank you!

ncik-roberts commented 4 months ago

In https://github.com/janestreet/merlin-jst/pull/73#issuecomment-2253475246 I suggested only omitting a ghost-location argument when it's None. Maybe that's preferable here too?

xvw commented 4 months ago

In janestreet/merlin-jst#73 (comment) I suggested only omitting a ghost-location argument when it's None. Maybe that's preferable here too?

Yep, I'll do a patch today! Thanks for the feedback.