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

Feature request: generate .merlin files #1

Closed
rgrinberg opened this issue Jan 28, 2017 · 9 comments
Closed

Feature request: generate .merlin files #1

rgrinberg opened this issue Jan 28, 2017 · 9 comments

Comments

@rgrinberg
Copy link
Member

This would be especially awesome if the janestreet packages were released with these .merlin files checked in to ease browsing code.

@ghost
Copy link

ghost commented Jan 30, 2017

Indeed, that's on the todo list. BTW I haven't looked at what merlin files look like, but I believe it should be straightforward to add, if you are interested in contributing I can give you some pointers

@bmillwood
Copy link

Doesn't jenga-rules already do this? e.g. https://github.com/janestreet/jenga-rules/blob/master/lib/root.ml#L4747

Or is the idea to make sure they're installed even when not using full jenga?

@ghost
Copy link

ghost commented Jan 30, 2017

The plan is to eventually stop supporting jenga-rules and have a jenga plugin that interprets the jbuilder rules directly. Maintaining our internal jenga rules to work in opam is a non-negligible amount of work and we need the jbuilder rules anyway, so extending jbuilder is the way to go

@rdavison
Copy link
Contributor

rdavison commented Feb 1, 2017

If no one wants to work on this, I'd be glad to take it on

@ghost
Copy link

ghost commented Feb 2, 2017

@rdavison that would be welcome! All the rules are in src/gen_rules.ml. The natural places to generate the .merlin files are in the library_rules and executables_rules functions, which generate the rules for libraries and executables.

To add a rule, you need to use add_rule <build array> where <build arrow> is a value of type (unit, unit) Build.t. Typically your rule will looks like this:

add_rule (
   <compute the contents of .merlin>
   >>>
   Build.echo (Path.relative dir ".merlin")
)

Build.echo path is an arrow that takes a string as input and print it to the given file. In both library_rules and executables_rules there is a let requires =... value that is an arrow of type (unit, Lib.t list) Build.t that resolves the dependencies. You can compose this arrow and use functions from the Lib module to get things such as the list of directories. For instance, if the merlin file contained a list of directories to include, one per line, you could do:

add_rule (
   requires
   >>>
   Build.arr (fun libs ->
      let paths = Lib.include_paths libs in
      String.concat ~sep:" " (List.map (Path.Set.elements paths) ~f:(fun path ->
         Path.reach path ~from:dir ^ "\n"))
  >>>
  Build.echo (Path.relative dir ".merlin")
)

Path.reach is used to turn a Path.t into a relative path starting from dir.

Note that this would generate .merlin files in the _build directory, but I imagine we want to generate them in the source directories, so we have to change this a bit. Moreover jbuilder supports building against multiple installation of OCaml simultaneously and we can only have one rule to produce a given target, so you'll need to do this:

match Path.extract_build_context dir with
| Some ("default", dir (* here dir is the path relative to `_build/default` *)) ->
   add_rule (...)
| _ -> ()

Once that is done, you'll be able to call jbuilder src/.merlin to generate src/.merlin. For later we'll need a more automated way to generate all the .merlin files, but we can do this after.

@rdavison
Copy link
Contributor

rdavison commented Feb 2, 2017

Sounds good, I'll play around with it a bit tonight and we'll see how far I get. Appreciate for the write up!

@rdavison
Copy link
Contributor

rdavison commented Feb 7, 2017

So just to leave some feedback and progress update, I'm still working on it, actually, and I've found a bit strange behavior that I'm not fully sure how to interpret. It appears that if I compose the let result = result ... with another arrow, nothing ever actually gets executed.

  let requires =
    requires ~dir ~dep_kind ~item:lib.name
      ~libraries:lib.libraries
      ~preprocess:lib.preprocess
      ~virtual_deps:lib.virtual_deps
  in

  add_rule (
    requires
    >>>
    Build.arr (fun libs ->
      print_endline "never executed"
    )

But digging deeper into the actual requires function I found that I'm able to insert some code slightly upstream and get some results.

    let requires ~dir ~dep_kind ~item ~libraries ~preprocess ~virtual_deps =
      let all_pps =
        Preprocess_map.pps preprocess
        |> Pp_set.elements
        |> List.map ~f:Pp.to_string
      in
      let vrequires = Lib_db.vrequires ~dir ~item in
      add_rule
        (Build.record_lib_deps ~dir ~kind:dep_kind (List.map virtual_deps ~f:Lib_dep.direct)
         >>>
         Build.fanout
           (Lib_db.closure ~dir ~dep_kind libraries)
           (Lib_db.closed_ppx_runtime_deps_of ~dir ~dep_kind
              (List.map all_pps ~f:Lib_dep.direct))
         >>>
         Build.arr (fun (libs, rt_deps) ->
           Lib.remove_dups_preserve_order (libs @ rt_deps))
         >>>
  -->    Build.fanout
  -->      (Build.arr (fun libs ->
  -->       let paths = Lib.include_paths libs in
  -->       let concat =
  -->         String.concat ~sep:" " (List.map (Path.Set.elements paths) ~f:(fun path ->
  -->         Path.reach path ~from:dir ^ "\n"))
  -->       in
  -->       concat) >>> Build.echo (Path.relative dir ".merlin"))
  -->      (Build.store_vfile vrequires)
  -->    >>>
  -->    Build.arr (fun ((),()) -> ()));
      Build.vpath vrequires

Also, it appears that in order to get completion within a library, one must include an -open flag in the .merlin file. So we will need some way to distinguish unix from jbuilder in this case since jbuilder will need to be implicitly opened inside the library.

S .
B ../_build/default/src
PKG unix jbuilder
FLG -open Jbuilder```

@rdavison
Copy link
Contributor

rdavison commented Feb 7, 2017

Update: After writing this, I took a second look at your original write-up and noticed that you mentioned that in order to generate the files, then jbuilder would have to be called with the target, which for example could be jbuilder src/.merlin. I moved the new code from the requires function back out to the library_rules function, tested that calling convention, and it correctly generated files in the _build subdirectory.

@ghost
Copy link

ghost commented Feb 25, 2017

This is now done

This issue was closed.
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

3 participants