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

Add merlin locate tests and add build dirs of modules to Merlin so copy# is visible. #10381

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jonahbeckford
Copy link
Collaborator

@jonahbeckford jonahbeckford commented Apr 3, 2024

This is a partial follow up to #10178

+bugfix: Adds the build directory of modules to the Merlin source directories so copy# can be followed. Without this modification (merlin.ml:Unprocessed.process) the test

$ cat _build/default/copyroundtrip/copy_of_a.ml
# 1 "copyroundtrip/a.ml"
let value = 3
$ cat ./$LIBNAME/b.ml | ocamlmerlin single locate -look-for ml -position 1:10 -filename b.ml | jq .value
{
"file": "$TESTCASE_ROOT/_build/default/copyroundtrip/copy_of_a.ml",
"pos": {
"line": 1,
"col": 0
}
}

would be

 $ cat ./$LIBNAME/b.ml | ocamlmerlin single locate -look-for ml -position 1:10 -filename b.ml | jq .value
 "'Copy_of_a' seems to originate from 'Copy_of_a' whose ML file could not be found"

+Adds a test for basic [locate] command

+Adds a "copyroundtrip" test for a [locate] command whose source is from a [copy#] stanza

  • Upstream issue: ocamlmerlin should read copy# line header and report the original file not the copied file.

+Adds merlin package to TEST_DEPS so that ocamlmerlin binary can be used in tests

+bugfix: Adds the build directory of modules to the Merlin source directories so copy# can be followed.

+ Adds a test for basic [locate] command

+ Adds a "copyroundtrip" test for a [locate] command whose source is from a [copy#] stanza
  * Upstream issue: `ocamlmerlin` should read copy# line header and report the original file not the copied file.

+ Adds `merlin` package to TEST_DEPS so that `ocamlmerlin` binary can be used in tests

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>
+Correct melange merlin tests

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>
@jonahbeckford jonahbeckford force-pushed the fix-merlin-add-source-of-copied-file branch from 2a2b656 to b98f5f7 Compare April 3, 2024 23:47
@jonahbeckford
Copy link
Collaborator Author

What is missing: the last step needs to be something like making sure the final file is not In_build_dir.

Today, Merlin does not seem to respect the line header, so it give back a location in In_build_dir. That path should be skipped.

Tomorrow, when Merlin respects the line header and gives back a location in the source tree, that path should be used.

I think that is reasonable logic (although I'm not sure where it goes).

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.

None yet

1 participant