Skip to content

Fix Merlin file loading #12621

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.

Sign up for GitHub

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

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

Fix Merlin file loading #12621

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8eb27b7
adding testcase for merlin config with multiple executables.
Durbatuluk1701 Oct 24, 2025
56e45f1
factor out common logic for merlin file loading
Durbatuluk1701 Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 34 additions & 11 deletions bin/ocaml/ocaml_merlin.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,44 @@ end = struct
let load_merlin_file file =
(* We search for an appropriate merlin configuration in the current
directory and its parents *)
let basename = Path.Build.set_extension file ~ext:"" |> Path.Build.basename in
let good_names =
List.map
~f:(fun pref -> Printf.sprintf "%s-%s" pref basename)
[ "lib"; "exe"; "melange" ]
in
let rec find_closest path =
let merlin_paths = get_merlin_files_paths path in
(* Now we want to look for:
1. an exact match
2. an approximate match
3. recursing to parent directory
*)
match
get_merlin_files_paths path
|> List.find_map ~f:(fun file_path ->
(* FIXME we are racing against the build system writing these
files here *)
match Merlin.Processed.load_file file_path with
| Error msg -> Some (Merlin_conf.make_error msg)
| Ok config -> Merlin.Processed.get config ~file)
List.find merlin_paths ~f:(fun p ->
List.mem good_names (Path.basename p) ~equal:String.equal)
with
| Some p -> Some p
| Some p ->
(* Found exact match: we are done *)
(match Merlin.Processed.load_file p with
| Error msg -> Some (Merlin_conf.make_error msg)
| Ok config -> Merlin.Processed.get config ~file)
| None ->
(match Path.Build.parent path with
| None -> None
| Some dir -> find_closest dir)
(* looking for approximate match *)
(match
List.find_map merlin_paths ~f:(fun file_path ->
(* FIXME we are racing against the build system writing these
files here *)
match Merlin.Processed.load_file file_path with
| Error msg -> Some (Merlin_conf.make_error msg)
| Ok config -> Merlin.Processed.get config ~file)
Copy link
Collaborator

@voodoos voodoos Oct 28, 2025

Choose a reason for hiding this comment

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

Let's find a way not to repeat that code.

Copy link
Author

@Durbatuluk1701 Durbatuluk1701 Oct 28, 2025

Choose a reason for hiding this comment

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

Sounds good, I think I got it unless you want it even further abstracted

with
| Some p -> Some p
| None ->
(* Otherwise, recurse upwards *)
(match Path.Build.parent path with
| None -> None
| Some dir -> find_closest dir))
in
match find_closest (Path.Build.parent_exn file) with
| Some x -> x
Expand Down
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/dune-project
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 1.10)
Empty file added test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/multi-exes/bin/b.ml
View file
Open in desktop
Empty file.
Empty file added test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/multi-exes/bin/c.ml
View file
Open in desktop
Empty file.
6 changes: 6 additions & 0 deletions test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/multi-exes/bin/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
(executable
(name b))

(executable
(name c)
(libraries lib))
2 changes: 2 additions & 0 deletions test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/multi-exes/lib/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
(library
(name lib))
Empty file added test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/multi-exes/lib/lib.ml
View file
Open in desktop
Empty file.
15 changes: 15 additions & 0 deletions test/blackbox-tests/test-cases/merlin/merlin-multi-exes.t/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
$ ocamlc_where="$(ocamlc -where)"
$ export BUILD_PATH_PREFIX_MAP="/OCAMLC_WHERE=$ocamlc_where:$BUILD_PATH_PREFIX_MAP"

$ FILE=$PWD/multi-exes/bin/b.ml
$ printf "(4:File%d:%s)" ${#FILE} $FILE | dune ocaml-merlin
((5:ERROR71:No config found for file multi-exes/bin/b.ml. Try calling 'dune build'.))

$ dune build @check

$ printf "(4:File%d:%s)" ${#FILE} $FILE | dune ocaml-merlin | sed -E "s/[[:digit:]]+:/\?:/g"
((?:INDEX?:$TESTCASE_ROOT/_build/default/multi-exes/bin/.c.eobjs/cctx.ocaml-index)(?:INDEX?:$TESTCASE_ROOT/_build/default/multi-exes/bin/.b.eobjs/cctx.ocaml-index)(?:INDEX?:$TESTCASE_ROOT/_build/default/multi-exes/lib/.lib.objs/cctx.ocaml-index)(?:STDLIB?:/OCAMLC_WHERE)(?:SOURCE_ROOT?:$TESTCASE_ROOT)(?:EXCLUDE_QUERY_DIR)(?:B?:$TESTCASE_ROOT/_build/default/multi-exes/bin/.b.eobjs/byte)(?:S?:$TESTCASE_ROOT/multi-exes/bin)(?:FLG(?:-w?:@[email protected]@30..39@[email protected]@[email protected]?:-strict-sequence?:-strict-formats?:-short-paths?:-keep-locs?:-g))(?:UNIT_NAME?:b))

$ FILE=$PWD/multi-exes/bin/c.ml
$ printf "(4:File%d:%s)" ${#FILE} $FILE | dune ocaml-merlin | sed -E "s/[[:digit:]]+:/\?:/g"
((?:INDEX?:$TESTCASE_ROOT/_build/default/multi-exes/bin/.c.eobjs/cctx.ocaml-index)(?:INDEX?:$TESTCASE_ROOT/_build/default/multi-exes/bin/.b.eobjs/cctx.ocaml-index)(?:INDEX?:$TESTCASE_ROOT/_build/default/multi-exes/lib/.lib.objs/cctx.ocaml-index)(?:STDLIB?:/OCAMLC_WHERE)(?:SOURCE_ROOT?:$TESTCASE_ROOT)(?:EXCLUDE_QUERY_DIR)(?:B?:$TESTCASE_ROOT/_build/default/multi-exes/bin/.c.eobjs/byte)(?:B?:$TESTCASE_ROOT/_build/default/multi-exes/lib/.lib.objs/byte)(?:S?:$TESTCASE_ROOT/multi-exes/bin)(?:S?:$TESTCASE_ROOT/multi-exes/lib)(?:FLG(?:-w?:@[email protected]@30..39@[email protected]@[email protected]?:-strict-sequence?:-strict-formats?:-short-paths?:-keep-locs?:-g))(?:UNIT_NAME?:c))
Loading