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

utop + jbuilder integration #114

Closed
avsm opened this Issue Jun 1, 2017 · 17 comments

Comments

Projects
None yet
5 participants
@avsm
Copy link
Member

avsm commented Jun 1, 2017

While working on Real World OCaml, @yminsky and I were discussing the possibility of integrating utop directly into jbuilder so that the interactive toplevel had the same set of warnings as those jbuilder uses. This would make our interactive examples much easier to work with.

I imagine something like jbuilder top that drops the user into a utop instance that has access to the same context as jbuilder exec, with locally built modules and so on available there.

@diml

This comment has been minimized.

Copy link
Member

diml commented Jun 1, 2017

Do you mean without doing #require ...?

@bikallem

This comment has been minimized.

Copy link
Contributor

bikallem commented Jun 1, 2017

Indeed, this feature would be super useful and would make .ocamlinit cruft mostly redundant. 💯

@diml

This comment has been minimized.

Copy link
Member

diml commented Jun 2, 2017

So the following can be done quite easily: if you run jbuilder top in a directory containing libraries, you get a toplevel with all these libraries preloaded and ready to use.

For the warnings it's a bit more complicated as jbuilder currently just knows about flags and not warnings specifically. Moreover once we'll get warnings we'll want other options such as -strict-blah as well and the toplevel doesn't understand all ocamlc flags. We'd need to hack something in utop. Jbuilder could only pass an approximation as it does for merlin at the moment.

@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Jun 27, 2017

So the following can be done quite easily: if you run jbuilder top in a directory containing libraries, you get a toplevel with all these libraries preloaded and ready to use.

I'd be interested on working on that @diml. Could you give this a quick sketch?

@diml

This comment has been minimized.

Copy link
Member

diml commented Jun 27, 2017

Sure. One question is whether we want to link custom utops or simply call utop with the right cmas on the command line. I think the former is slightly simpler to implement so we can try that. This is what needs to be done:

  1. in every directory containing at least one library, add a rule to build an entry point for the toplevel as .utop.ml. This file should do two things:
  • set Clflags.include_dirs appropriately
  • call UTop_main.main
  1. add a rule to link a custom toplevel as .utop.bc. You can use the function build_exe for that. Let me know if you need clarification about the argument it takes

  2. add a top subcommand that starts by building the appropriate .utop.bc and then run it

@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Jun 29, 2017

@diml thanks

Regarding 1, Seems like this would limit the support to loading a single library (and its dependencies at once). While not awful, loading more than 1 library (usually all of them) is a common enough use case. Anyway we can support that? I realize that this might require generating build rules from the command invocation itself but perhaps it's worth it? This isn't an essential thing to worry about now though

Some bike shedding about 3: do you mean a utop subcommand? This really will only work for utop and will require it as a dependency so might as well make the command reflect that. Or is there a possibility this will be extended for regular top levels as well?

@diml

This comment has been minimized.

Copy link
Member

diml commented Jun 29, 2017

Yh, I was actually thinking of linking statically all the libraries defined in the current directory.

I realize that this might require generating build rules from the command invocation itself but perhaps it's worth it?

What do you mean by generating build rules from the command invocation?

Some bike shedding about 3: do you mean a utop subcommand? This really will only work for utop and will require it as a dependency so might as well make the command reflect that. Or is there a possibility this will be extended for regular top levels as well?

Yh, I agree a utop subcommand is clearer

@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Jun 29, 2017

What do you mean by generating build rules from the command invocation?

Never mind. Initially, I thought you meant that every .utop.ml would only be able produce a top level for 1 library. So I thought to support multiple library we'd need to generate a more complex .utop.ml to load multiple libraries. And obviously that would depend on which library requested a top level for. We don't need any of this stuff if we just load 1 library for now.

@rgrinberg rgrinberg self-assigned this Jun 29, 2017

@rgrinberg rgrinberg referenced this issue Jul 9, 2017

Merged

utop subcommand #183

2 of 2 tasks complete
@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Jul 11, 2017

@diml One more thing to consider: It would be nice if there was a way to automatically load libraries that install toplevel printers. In findlib, this can be done with predicates, e.g. archive(byte,toploop) += "tyxml_top.cma". If jbuilder could generate this line for libraries marked as toplevel only, and also use this mechanism when building toplevels itself ($ jbuilder utop) that would make for a smooth user experience.

@diml

This comment has been minimized.

Copy link
Member

diml commented Jul 14, 2017

Possibly, but frankly this mechanism is a bit heavy. Now that we have annotations, we should handle toplevel printers using annotations and let the toplevel use them automatically. This would avoid having to write these boilerplate libraries manually

@dbuenzli

This comment has been minimized.

Copy link

dbuenzli commented Jul 15, 2017

@rgrinberg You should always allow the user to load the library without loading its toplevel support so the findlib mecanism you suggest should not be used in my opinion (this was discussed here).

The scheme I implemented in all my packages doesn't need findlib predicates, of course what @diml proposes would be much nicer to have. But while what he propose with solve say 98% of the cases we may still need other init bits so some kind of support to evaluate phrases for a library on toplevel load needs to occur somewhere.

Regarding the scheme I propose you can have a look how it is implemented for gg here for the META, here and here.

Note that odig's toplevel loaders eschew loading the pkg_top libraries and simply read a pkg_top.ml file if pkg.cma is loaded and the file exists. With that convention and what @diml proposes we will no longer have to pollute the package name hierarchy with pkg.top libraries.

@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Jul 16, 2017

Now that we have annotations, we should handle toplevel printers using annotations and let the toplevel use them automatically

Could you expand on this a little bit? I don't know enough about the toplevel to know how this would work.

You should always allow the user to load the library without loading its toplevel support so the findlib mecanism you suggest should not be used in my opinion (this was discussed here).

Shouldn't it be easy enough to have findlib add another #require-like directive that doesn't set the toploop predicate?

In any case, your proposal seems good and I don't mind implementing this without relying on findlib. But it should be easy enough for users to open a toplevel with all the pretty printers installed, so I think we'd still need a directive that respects this init scheme.

@dbuenzli

This comment has been minimized.

Copy link

dbuenzli commented Jul 16, 2017

But it should be easy enough for users to open a toplevel with all the pretty printers installed, so I think we'd still need a directive that respects this init scheme.

odig's toplevel printers will do this automatically unless prevented via the init argument. In ocamlfind users should simply #require the pkg.top library rather than pkg, I'm not sure a new directive is really needed.

@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Jul 16, 2017

Hmm, but I thought the goal was to get rid of these pkg.top libraries completely, so that they don't pollute the package name hierarchy. Or do you mean, these packages will no longer be necessary for odig users?

Btw, I was completely unaware that odig had anything to do with toplevels until now. But it would be good if jbuilder was compatible with odig in general. Is there anything that needs to be done there?

@diml

This comment has been minimized.

Copy link
Member

diml commented Jul 17, 2017

Now that we have annotations, we should handle toplevel printers using annotations and let the toplevel use them automatically

Could you expand on this a little bit? I don't know enough about the toplevel to know how this would work.

For instance you could have an annotation like this in your mli files:

val pp : Format.formatter -> t -> unit [@@toplevel_printer]

and then the toplevel would automatically do #install_printer Foo.pp when loading foo.cmi. This requires some changes in OCaml itself.

Otherwise I'm happy to use the same convention as odig.

@dbuenzli

This comment has been minimized.

Copy link

dbuenzli commented Jul 18, 2017

Or do you mean, these packages will no longer be necessary for odig users?

Yes because of the loading convention. Follow and read the links to documentation in my earlier message.

@rgrinberg

This comment has been minimized.

Copy link
Member

rgrinberg commented Apr 23, 2018

This was a fixed a long time ago. There are more specific tickets on how to make toplevels work better.

@rgrinberg rgrinberg closed this Apr 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment