ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.6k stars 397 forks source link

[merlin] preprocess + run results in -pp and not -ppx #4169

Closed gares closed 2 years ago

gares commented 3 years ago

It seems that while ocamlc is happy to receive marshaled AST, ocamllsp(merlin) is not happy receiving mashaled AST if a -pp is given (a -ppx would work).

At the same time preprocess with a user action generates a merlin config which passes -pp, and there seem to be no way to override this decision.

Expected Behavior

(preprocess (per_module  ((action (run "bla")) m)))

should/could generate a -ppx directive for merlin

Actual Behavior

generates always a -pp directive for merlin

Reproduction

opam list | grep serv
# ocaml-lsp-server         1.4.0       LSP Server for OCaml
opam list | grep dune
# dune                     2.8.2       Fast, portable, and opinionated build system
# dune-build-info          2.8.2       Embed build informations inside executable
# dune-configurator        2.8.2       Helper library for gathering system configuration
# dune-private-libs        2.6.2       Private libraries of Dune
git clone https://github.com/LPCIC/elpi
git checkout rm-merlin-hack
dune build
code . # open src/data.ml or src/runtime_trace_on.ml

See also https://github.com/ocaml/dune/issues/1212 , CC @voodoos

gares commented 3 years ago

IMO the easiest fix is to have ocamllsp/merlin check if the input is text or not (eg if it begins with Caml1999M023). A way to declare what a preprocess+run generates (text or AST) would also work for me.

In elpi I have 2 use cases for run:

rgrinberg commented 3 years ago

To clarify, ocamlc -pp ./foo.exe work if foo.exe produces a marshalled ast? If it does, then we should just fix merlin to accept the marshalled ast here.

gares commented 3 years ago

This is my understanding.

gares@ollypat:~/LPCIC/elpi$ file _build/default/src/data.pp.ml
_build/default/src/data.pp.ml: OCaml abstract syntax tree implementation file (Version 023)
gares commented 3 years ago

My only concern is that I don't know if the flags -pp and -ppx have other side effects.

Kakadu commented 3 years ago

To clarify, ocamlc -pp ./foo.exe work if foo.exe produces a marshalled ast? If it does, then we should just fix merlin to accept the marshalled ast here.

Yes, it does, @rgrinberg

ejgallego commented 3 years ago

I understand this is preventing @gares from using 2.8 , as dune ocaml-merlin is broken in his setup and he cannot override the .merlin file; I'm not sure I fully understand all the details in this issue, what would be the best path to fix it?

voodoos commented 3 years ago

It is possible to "override the .merlin" file. If a .merlin file is present in the source folder it will take priority on the dune file and dot-merlin-reader will be used.

You can copy the .merlin file generated by an older dune or write it manually in the project sources and commit that to the git directory.

Dune 2.9 will ship with a new command line tool to dump Merlin configuration in .merlin format to make this easier.

gares commented 3 years ago

I could then "sed" the merling file replacing -pp with -ppx but the fix could also be done once and forall on the merlin side, eg fix https://github.com/ocaml/merlin/issues/1246

rgrinberg commented 2 years ago

I believe this should be fixed upstream in merlin.