On Windows, generated .merlin ppx path incorrectly uses backslashes

[leviroth]

I have a jbuild:

(jbuild_version 1)

(executables
 ((names        (example))
  (flags (:standard -short-paths))
  (preprocess (pps (ppx_jane)))))

with example.ml:

open Base

let f = [%compare.equal: int]

This builds just fine. However, the generated .merlin has the following line:

FLG -ppx "C:\Users\levim\src\ppx-example\_build/default/.ppx/ppx_jane/ppx.exe --as-ppx"

This results in merlin trying and failing to find the ppx at:

C:.\\Userslevimsrcppx-example_build\\default\\.ppx\\ppx_jane\\ppx.exe

This apparently can be fixed by manually modifying the \ in the generated .merlin either to / or \\.

Related to #201.

[rgrinberg]

cc @dra27

[leviroth]

Or, maybe merlin itself should be responsible for reading the \ correctly, without escaping?

[rgrinberg]

Maybe :) Let's cc @let-def as well.

[rgrinberg]

@diml @dra27 Can we change the generation to use / on win32 as well?

[ghost]

I let @dra27 comment on whether it's preferable to use / or \\, but yh we should make sure we generate something merlin can understand

[ghost]

I suppose we should add Path.quote_for_shell that would take care of all this

[ghost]

@dra27 do you have time to look at this issue? I believe that we'll need to sort that out for #193 given that the ppx rewriter will be passed to the compiler via -ppx which takes a string that is then passed to Sys.command.

[nojb]

Not sure that dune is the right place to fix this. The code that reads this option in merlin unconditionally interprets \ to mean "unquote next char", which is wrong on Windows. See https://github.com/ocaml/merlin/blob/13210bd5638a59f915b854d838a07f0344c713ff/src/utils/std.ml#L701-L706

[ghost]

Do you mean dune isn't the right place to fix this?

[nojb]

Do you mean dune isn't the right place to fix this?

Yes, that's my understanding.

[ghost]

Ok. Is there a ticket on merlin about this BTW?

[mlasson]

On merlin sides, it seems that a correct way to implement this side would be to have some kind of external program that prints its arguments (something like "let () = Array.iter print_endline Sys.argv") and call this program with "Sys.command" for each "FLG " lines in the .merlin in order to split the arguments like the "host shell". But that would be an overkill, no ?

Edit: Humm.. actually, it will be probably not escape the result back to work correctly with the Sys.command that will actually call the ppx. This feel hard.

In the mean time, in dune, we could either replace the backslash in the executable_name by forward slashes which would be consistent anyway with the rest of the path from the root.

[ghost]

In the mean time, in dune, we could either replace the backslash in the executable_name by forward slashes which would be consistent anyway with the rest of the path from the root.

That seems fine to me