Add command wrapping ocamlformat

[gridbugs]

Adds a command dune tools exec ocamlformat which downloads and builds ocamlformat as a dev tool before running it, passing all positional arguments to the ocamlformat executable. This is intended to be run by text editors.

[gridbugs]

I found an issue with this where if opam is installed but no switch is set then dune tools exec ocamlformat won't fall back to a globally-installed instance of ocamlformat. Will fix.

[gridbugs]

I found an issue with this where if opam is installed but no switch is set then dune tools exec ocamlformat won't fall back to a globally-installed instance of ocamlformat. Will fix.

This is now fixed.

[gridbugs]

I don't understand why the opam switch create test is failing. I can reproduce it on the main branch locally but error doesn't make sense to me based on the vendored copy of pp. In any case it's not related to this change.

[rgrinberg]

I think we just need to update our vendored copy of pp

[rgrinberg]

I'm not sure that we need this fallback. Or perhaps put differently, I'm not sure dune should be responsible for it. I could imagine that editors might want to perhaps suggest to the user if they would like to switch to dune's package management to make this command work, or perhaps tell them there's an opam fallback available. Or perhaps the user should have a way to prefer to use opam and dune shouldn't even try package management at all.

[gridbugs]

I guess the question is how much sophistication we expect from editor plugins. Today in emacs and neovim there's a way to set a command to run to invoke ocamlformat (overriding the default command which is just ocamlformat). My goal adding fallback behaviour to dune was to avoid needing to change editor plugins to become explicitly aware of dune dev tools. I claim that centralizing fallback behaviour within dune rather than implementing it in each ocamlformat editor plugin will be less work and easier to maintain. Centralizing this behaviour in dune also means that users can try out dune-managed ocamlformat (and soon ocamllsp too) today by just updating their editor config and don't need to wait for support to land in their editor plugins.

[rgrinberg]

It would be easier to maintain if there was one fallback behavior which was correct in all cases. I don't think that's the case though. In the vscode plugin for example, a sandbox is selected explicitly, and once the user selects the sandbox all fallback options don't really make sense.

With that being said, you can keep the current behavior, but it's very likely that it will be subject to change once we evolve this feature.

[gridbugs]

Ah I did not realize that vscode's ocaml plugin doesn't allow the user to customize the commands used to run ocamlformat or ocamllsp and has a built-in understanding of opam switches. From playing around with it a bit it seems like the plugin will need to be updated to allow dune to manage these tools no matter what the ui looks like on the dune side.

For emacs and vim the plugins for lsp and ocamlformat are lower level and let you customize the command that runs the tool. Generally I have found it necessary to customize the command to be opam exec <tool> -- and I'm trying to make it so all users need to do to adopt dune dev tools is change the command to dune tools exec <tool> -- without needing to add sophistication to plugins.

[rgrinberg]

Right but I think that communicating the editor a "dev environment" is better than having to teach each tool about dune tools and its fallback behavior. Configuring every single command separately inside the editor rather than just telling it "here's a dev environment, fetch what you need" seems a bit simpler. For example, ocamllsp needs to invoke ocamlformat. Do you expect it to be modified to run dune tools

This is what I implemented in vscode from my experience learning from esy at least.

[gridbugs]

I think that makes sense for vscode but I'm not so sure about other editors. For example in neovim there is no need for an ocaml-specific lsp plugin. The general lsp client plugin just requires a command to start the lsp server for each language, so for ocaml you can just tell it to run opam exec ocamllsp --. There's also no plugin necessary for ocamlformat. I use a more general formatting plugin called "neoformat" and configure it with a command to run to format files with an ocaml extension.

I agree that the fact that ocamllsp invokes ocamlformat does complicate the editor integration story a bit but I'd be interested to find a way to resolve this without needing to add sophistication to editor plugins. One option would be to add an argument to ocamllsp for overriding the command to run ocamlformat.

[rgrinberg]

Btw how does your proposal work with other binaries in ocamlformat? For example, ocamllsp also uses ocamlformat-rpc to format tooltips.

One option would be to add an argument to ocamllsp for overriding the command to run ocamlformat.

I think that's exactly why going down this route is not what we'd like to do. It is advantageous for dev tools to be able to use each other to provide additional functionality. We shouldn't be introducing barriers for this to work. Adding new dev tools that leverage existing ones should be something to facilitate instead.

Here's an alternative proposal, we create a "dev" environment that sets the PATH to include all dev tools. Then, we can run any dev tool as dune tools exec -- <tool name>.

[rgrinberg]

Created a new ticket to track this: #10964

[gridbugs]

Here's an alternative proposal, we create a "dev" environment that sets the PATH to include all dev tools. Then, we can run any dev tool as dune tools exec -- .

I like this idea. Rather than needing to wrap every tool with an explicit command, we just make dune tools exec -- <command> execute <command> in an environment with all the dev tools in PATH. It would also need to make sure that if the command is running a dev tool that isn't currently installed, then it gets locked and installed.

[gridbugs]

I've updated this PR to remove any fallback behaviour. Changes that extend environments to allow dev tools to call each other can come in a later change.

[moyodiallo]

I've updated this PR to remove any fallback behaviour. Changes that extend environments to allow dev tools to call each other can come in a later change.

The dev tools calling each other is temporary ?
I wonder when we start having different versions of ocamlformat in a global cache to prevent rebuilding ocamlformat when switching on different projects, how we could deal with it.

[gridbugs]

The dev tools calling each other is temporary ?

Why would it be temporary?

[gridbugs]

I wonder when we start having different versions of ocamlformat in a global cache to prevent rebuilding ocamlformat when switching on different projects, how we could deal with it.

We'll always have the option to extend the environment of dev tools to include all the other dev tools in PATH. That's what my ocamllsp wrapper script does, but this behavour could also be built in to dune (not in this PR though).

[gridbugs]

Is anything else required before this can be merged?

[gridbugs]

Ah looks like it doesn't build after rebasing. Will fix.

[gridbugs]

Ok I fixed the build error. Is anything else needed before this can be merged?