Skip to content
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

Backport 1727 #56

Merged
merged 39 commits into from
Apr 30, 2024
Merged

Backport 1727 #56

merged 39 commits into from
Apr 30, 2024

Conversation

poechsel
Copy link
Contributor

"Backport" ocaml/merlin#1727 which is a pre-requisite of getting occurences working.

This backport was done by cherry-picking each commit of 1727 one after the other. Tests outputs were manually inspected to check if the final diff is consistent with the one of the upstream PR.

The main difference with upstream PR resides in the last two commits: one is moving some code in its own files, the other is making sure that occurrences can rely on cms files.

Copy link
Contributor

@liam923 liam923 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -31,261 +31,20 @@ open Std
let last_location = ref Location.none

let {Logger. log} = Logger.for_section "locate"
module File : sig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a technical reason for moving these modules into separate files or is it just for better code structuring? If the latter, I'm hesitant to diverge from upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the commit adding Artifact.ml / File.ml and instead exposed Artifact in Locate's mli

Signed-off-by: Pierre Oechsel <[email protected]>
Signed-off-by: Pierre Oechsel <[email protected]>
Signed-off-by: Pierre Oechsel <[email protected]>
Signed-off-by: Pierre Oechsel <[email protected]>
Signed-off-by: Pierre Oechsel <[email protected]>
Signed-off-by: Pierre Oechsel <[email protected]>
@liam923 liam923 mentioned this pull request Apr 30, 2024
@liam923 liam923 merged commit 3ecdab7 into main Apr 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants