fix: improve error messages when merlin is absent

[rgrinberg]

thanks @let-def

[ulugbekna]

What's "merlin-extend reason"? Something like "install merlin-extend package" or something in that spirit would be more understandable I think.

[rgrinberg]

reason is an additional package I'm proposing to the user to install.

[ulugbekna]

On a second thought, this results in Initial, Exn -> Initial, which shouldn't be the case.

I had a throwing code action now, which resulted in assertion failure from | Error Initial -> assert false

I think it would also make sense if the monoid combined all exceptions rather than taking the latest one.

Irrelevant but curious, the code action threw this

[Trace - 2:10:58 AM] Received response 'textDocument/codeAction - (39)' in 2ms. Request failed: uncaught exception (-32603).
Error data: {
    "exn": "File \"ocaml-lsp-server/vendor/merlin/src/ocaml/utils/local_store.ml\", line 53, characters 2-8: Assertion failed",
    "backtrace": "Raised at Ocaml_utils__Local_store.with_store in file \"ocaml-lsp-server/vendor/merlin/src/ocaml/utils/local_store.ml\", line 53, characters 2-39\nCalled from Mocaml.new_state in file \"ocaml-lsp-server/vendor/merlin/src/kernel/mocaml.ml\", line 12, characters 2-70\nCalled from Mpipeline.Cache.get in file \"ocaml-lsp-server/vendor/merlin/src/kernel/mpipeline.ml\", line 60, characters 18-37\nCalled from Mpipeline.process in file \"ocaml-lsp-server/vendor/merlin/src/kernel/mpipeline.ml\", line 141, characters 14-30\nCalled from Ocaml_lsp_server__Action_type_annotate.code_action.(fun) in file \"ocaml-lsp-server/src/code_actions/action_type_annotate.ml\", line 57, characters 25-68\nCalled from Merlin_utils__Std.let_ref in file \"ocaml-lsp-server/vendor/merlin/src/utils/std.ml\", line 686, characters 8-12\nRe-raised at Merlin_utils__Std.let_ref in file \"ocaml-lsp-server/vendor/merlin/src/utils/std.ml\", line 688, characters 30-39\nCalled from Merlin_utils__Misc.try_finally in file \"ocaml-lsp-server/vendor/merlin/src/utils/misc.ml\", line 45, characters 8-15\nRe-raised at Merlin_utils__Misc.try_finally in file \"ocaml-lsp-server/vendor/merlin/src/utils/misc.ml\", line 62, characters 10-24\nCalled from Stdlib__Fun.protect in file \"fun.ml\", line 33, characters 8-15\nRe-raised at Stdlib__Fun.protect in file \"fun.ml\", line 38, characters 6-52\nCalled from Mocaml.with_state in file \"ocaml-lsp-server/vendor/merlin/src/kernel/mocaml.ml\", line 18, characters 8-38\nRe-raised at Mocaml.with_state in file \"ocaml-lsp-server/vendor/merlin/src/kernel/mocaml.ml\", line 20, characters 42-53\nCalled from Stdune__Exn_with_backtrace.try_with in file \"submodules/dune/otherlibs/stdune/exn_with_backtrace.ml\", line 9, characters 8-12\nRe-raised at Stdune__Exn.raise_with_backtrace in file \"submodules/dune/otherlibs/stdune/exn.ml\", line 36, characters 27-56\nCalled from Fiber.O.(>>|).(fun) in file \"submodules/dune/src/fiber/fiber.ml\", line 338, characters 36-41\nCalled from Fiber.Execution_context.run_jobs in file \"submodules/dune/src/fiber/fiber.ml\", line 218, characters 8-13\nRe-raised at Stdune__Exn.raise_with_backtrace in file \"submodules/dune/otherlibs/stdune/exn.ml\", line 36, characters 27-56\nCalled from Fiber.O.(>>|).(fun) in file \"submodules/dune/src/fiber/fiber.ml\", line 338, characters 36-41\nCalled from Fiber.Execution_context.run_jobs in file \"submodules/dune/src/fiber/fiber.ml\", line 218, characters 8-13\n"
}

[ulugbekna]

Something like this perhaps?

  let combine x y =
    match (x, y) with
    | Initial, _ -> y (* [Initial] cedes to any *)
    | _, Initial -> x
    | _, Exn _ -> y (* [Exn] takes over any; the right one wins - why? *) 
    | Exn _, _ -> x
    | Need_merlin_extend _, Need_merlin_extend _ -> y

Another easy way would be to define comparison function on variants and simply always take max of them

[rgrinberg]

Something like this perhaps?

Yeah that looks fine.

Another easy way would be to define comparison function on variants and simply always take max of them

Writing the comparison function is just as much work so I think the current approach is better because it's more direct.