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

Isolate configuration files #393

Closed
emillon opened this issue Sep 4, 2018 · 6 comments
Closed

Isolate configuration files #393

emillon opened this issue Sep 4, 2018 · 6 comments
Assignees

Comments

@emillon
Copy link
Collaborator

emillon commented Sep 4, 2018

Hello,

I am implementing an integration with dune (see ocaml/dune#1201).

If I understand it correctly, at the moment, the configuration is read in cascade from the closest .ocamlformat all the way up until a global .ocamlformat file.

In the context of dune, this can cause reproducibility problems, because in a dune project with no .ocamlformat file (or with a partial one), running ocamlformat on a file will read each developer's global configuration file, and will cause different results.

I'm not sure about the best technical solution, but a --root configuration option that never reads files above a particular directory seems enough. This option would also disable reading the global configuration file and from the environment as well.

What do you think about this?

Thanks!

@hhugo
Copy link
Collaborator

hhugo commented Sep 4, 2018

related to #391

@jberdine
Copy link
Collaborator

jberdine commented Sep 4, 2018

Yes, the current behavior of ocamlformat is the way it is just because it was a quick hack and I didn't know a good way to identify the root. Adding a command line option would be very clear, and work well in cases where it is called from something that knows the project root, but maybe would require too much interaction for the use case in #391. What do you think @hhugo, would just a --root option work there?

@hhugo
Copy link
Collaborator

hhugo commented Sep 4, 2018

providing a --root would probably work fine together with some logic in the editor mode.

@emillon
Copy link
Collaborator Author

emillon commented Sep 5, 2018

To recap I think that there are three use cases:

  • ocamlformat is invoked from dune. dune knows the root dir, so --root is enough, but detecting dune-project or .git might work as well (there are probably cases where it will be less precise).
  • ocamlformat is invoked from an editor. The current directory might be a good root in the case of TUI editors, but for GUI editors the current directory will be $HOME or something, so autodetection is probably required
  • ocamlformat is invoked by hand. This situation is pretty important since it might be the first interaction of a user with this program, and it will potentially happen outside a dune or git directory. Once again, autodetection should be good enough, but there might be surprising cases when a new user tries to configure ocamlformat and the configuration gets ignored.

@jberdine
Copy link
Collaborator

jberdine commented Sep 5, 2018

  • It seems clear that there should be a --root option for use when the caller knows how to set it.
  • I expect that for editors, CWD is very often $HOME or somewhere below the actual project root, seems too fragile to use to me.
  • What about also having a --disable-unless-project-config-found (hopefully with some better name) option that would work like in Improve: disable ocamlformat if no dot ocamlformat in the project #391, and the editor integrations would just need to be modified to pass this flag when performing format-on-save. My feeling in general is that command line options are more discoverable and debuggable than env vars, but otherwise don't have a strong view. The idea for this proposal is to avoid duplicating project detection in all the editor integrations but still leaving the default invocation by hand alone. Thoughts?

@gpetiot
Copy link
Collaborator

gpetiot commented Oct 8, 2018

Closed with #402

@gpetiot gpetiot closed this as completed Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants