Merge 5.2.0-minus11 #146
Merge 5.2.0-minus11 #146
Conversation
src/kernel/mocaml.ml
Outdated
| let to_unit_info name = | ||
| Some (name |> | ||
| Compilation_unit.of_string |> | ||
| Unit_info.make_dummy ~input_name:name) |
There was a problem hiding this comment.
I think this won't break anything, but using make_dummy scares me a bit. It seems like the type of thing that may cause something to break down the line. If we want to call Unit_info.make_with_known_compilation_unit, we can use config.query.filename to fill source_file.
I'm not sure about intf_or_impl - I don't know if Merlin knows that at this point (I'm not sure if Merlin decides this based on solely the filename or whether it's able to parse successfully). And I don't know that the file_prefix argument is for.
Maybe @lukemaurer can chime in on the significance of using make_dummy here, and what the deal is with file_prefix?
There was a problem hiding this comment.
Yes, I wanted @lukemaurer to review this piece. It was very low confidence.
There was a problem hiding this comment.
Yeah, if there's a real source file then we don't want to be using make_dummy if we can avoid it. make_with_known_compilation_unit makes a lot of sense to me. The intf_or_impl thing is annoying - wish I could remember exactly what it's used for
There was a problem hiding this comment.
I seem to have lost my permissions to push to this repo. @lukemaurer can you look at the below commit: liam923/merlin@4123c5a
Specifically, I'm not sure what to pass for the file_prefix to Unit_info.make_with_known_compilation_unit.
There was a problem hiding this comment.
Aha I've gotten my permissions back. Commit 4123c5a is pushed here now.
|
@goldfirere please review the commits pushed by me |
|
I have reviewed through "Add new ignored flags". Just waiting on @lukemaurer at this point. |
|
I'm merging despite the discussion around |
Bring Merlin up-to-speed with oxcaml version 5.2.0minus-11.