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

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

Closed
gares opened this issue Jan 28, 2021 · 9 comments
Closed

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

gares opened this issue Jan 28, 2021 · 9 comments

Comments

@gares
Copy link
Contributor

gares commented Jan 28, 2021

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 #1212 , CC @voodoos

@gares
Copy link
Contributor Author

gares commented Jan 28, 2021

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:

  • one is camlp5, which can print text, but then all locs are wrong (for error or info reporting)
  • another one is a true ppx, but I have to override a little the main, so I can't let dune take over it completely and use pps

@rgrinberg
Copy link
Member

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
Copy link
Contributor Author

gares commented Jan 28, 2021

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
Copy link
Contributor Author

gares commented Jan 28, 2021

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

@Kakadu
Copy link
Contributor

Kakadu commented Feb 1, 2021

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
Copy link
Collaborator

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
Copy link
Collaborator

voodoos commented Mar 24, 2021

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
Copy link
Contributor Author

gares commented Mar 24, 2021

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 ocaml/merlin#1246

@rgrinberg
Copy link
Member

I believe this should be fixed upstream in merlin.

@rgrinberg rgrinberg closed this as not planned Won't fix, can't repro, duplicate, stale Jul 28, 2022
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

No branches or pull requests

5 participants