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

conventional style and ocp-indent #817

Closed
hannesm opened this issue May 3, 2019 · 10 comments
Closed

conventional style and ocp-indent #817

hannesm opened this issue May 3, 2019 · 10 comments

Comments

@hannesm
Copy link

hannesm commented May 3, 2019

Hello, I'd really like to use ocamlformat. I'm fine with the conventional style, since that seems to be well-thought. I put profile=conventional in my .ocamlformat, and used dune build @fmt and dune promote to get an initial set of changes. I browsed a bit through the diff, and encountered the following indentation, which my ocp-indent (using default config, i.e. no .ocp-indent around) disagrees with. I then put ocp-indent-compat into .ocamlformat, and re-run the build and promote, but this did not lead to a different indentation.

Now, my question is whether there is a convenient way to have ocp-indent and ocamlformat agreeing on the indentation? Or is there a way to generate a .ocp-indent from the ocamlformat conventional profile?

Please take a look at the following function as an example (indentation of the false block in the pattern matching), its current layout:

let find_or_generate_key key_filename bits seed =
  Bos.OS.File.exists key_filename >>= function
  | true ->
    Bos.OS.File.read key_filename >>= fun data ->
    (try Ok (X509.Encoding.Pem.Private_key.of_pem_cstruct1 (Cstruct.of_string data)) with
     | _ -> Error (`Msg "while parsing private key file"))
  | false ->
    let key =
      let g =
        match seed with
        | None -> None
        | Some seed ->
          let seed = Cstruct.of_string seed in
          Some Nocrypto.Rng.(create ~seed (module Generators.Fortuna))
      in
      `RSA (Nocrypto.Rsa.generate ?g bits)
    in
    let pem = X509.Encoding.Pem.Private_key.to_pem_cstruct1 key in
    Bos.OS.File.write ~mode:0o600 key_filename (Cstruct.to_string pem) >>= fun () ->
    Ok key

ocamlformat, profile=conventional, ocp-indent-compat

let find_or_generate_key key_filename bits seed =
  Bos.OS.File.exists key_filename >>= function
  | true -> (
      Bos.OS.File.read key_filename >>= fun data ->
      try
        Ok
          (X509.Encoding.Pem.Private_key.of_pem_cstruct1
             (Cstruct.of_string data))
      with _ -> Error (`Msg "while parsing private key file") )
  | false ->
      let key =
        let g =
          match seed with
          | None -> None
          | Some seed ->
              let seed = Cstruct.of_string seed in
              Some Nocrypto.Rng.(create ~seed (module Generators.Fortuna))
        in
        `RSA (Nocrypto.Rsa.generate ?g bits)
      in
      let pem = X509.Encoding.Pem.Private_key.to_pem_cstruct1 key in
      Bos.OS.File.write ~mode:0o600 key_filename (Cstruct.to_string pem)
      >>= fun () -> Ok key

using ocp-indent on that block (1.7.0 no configuration file, i.e. default):

let find_or_generate_key key_filename bits seed =
  Bos.OS.File.exists key_filename >>= function
  | true -> (
      Bos.OS.File.read key_filename >>= fun data ->
      try
        Ok
          (X509.Encoding.Pem.Private_key.of_pem_cstruct1
             (Cstruct.of_string data))
      with _ -> Error (`Msg "while parsing private key file") )
  | false ->
    let key =
      let g =
        match seed with
        | None -> None
        | Some seed ->
          let seed = Cstruct.of_string seed in
          Some Nocrypto.Rng.(create ~seed (module Generators.Fortuna))
      in
      `RSA (Nocrypto.Rsa.generate ?g bits)
    in
    let pem = X509.Encoding.Pem.Private_key.to_pem_cstruct1 key in
    Bos.OS.File.write ~mode:0o600 key_filename (Cstruct.to_string pem)
    >>= fun () -> Ok key

Any hints would be appreciated on how to combine the usage of ocp-indent and ocamlformat.

@gpetiot
Copy link
Collaborator

gpetiot commented May 3, 2019

The ocp-indent-compat option only does minor tweaks to the formatting to help ocp-indent, e.g. for a type constraint it puts the break before : instead of after because it helps ocp-indent to have this symbol at the beginning of the line to have a nice indentation.

Regarding the support of ocp-indent options, so far we only support match_clause, I opened #476, more options can get supported in the future.

Since you apply ocp-indent after ocamlformat you need to have some appropriate ocp-indent configuration indeed, match_clause = 4 in your .ocp-indent file should be enough (it is the default used by ocamlformat but you need to specify it explicitly for ocp-indent).

For the moment there is no plan to automatically generate an .ocp-indent file but that's something we could do in the future.

@hannesm
Copy link
Author

hannesm commented May 4, 2019

@gpetiot thanks for your quick reply. Before I re-run my experiments again, is match_clause = 4 the only thing I'd need explicitly in .ocp-indent when using ocamlformat 0.9 with profile=conventional, or are you aware of further options which would be required to have ocamlformat and ocp-indent in sync?

@gpetiot
Copy link
Collaborator

gpetiot commented May 6, 2019

We didn't really extensively experiment with post-processing the ocamlformat output with ocp-indent so I unfortunately don't know which options would be required, (some people at Jane Street are doing it to some extent and we fix bugs when they are reported), that's something we need to do in the near future.

@hannesm
Copy link
Author

hannesm commented May 6, 2019

@gpetiot thanks again. Maybe I lack the understanding of how to use ocamlformat, my impression was that it is best run as a save-hook for OCaml files. During development I like to not manually indent my code, that's why I use ocp-indent. Now, if I work on code, certainly ocp-indent will have to deal with the ocamlformat-formatted code, and my goal is to avoid indentation changes. This means rather than post-processing, source file are processed by ocamlformat (on save) and ocp-indent (on edit/add/remove) consecutively.

@samoht noted mirage/ocaml-cstruct#234 (comment) he has such a setup, but given that the indentation of ocamlformat and ocp-indent is different, I wonder how that works in more detail.

@gpetiot
Copy link
Collaborator

gpetiot commented May 6, 2019

This workflow makes sense, but I need to practice it myself so that I can spot the inconsistencies between ocamlformat and ocp-indent, we definitely need to add more tests covering this workflow in ocamlformat.

@avsm
Copy link
Contributor

avsm commented May 14, 2019

cross-linking this to #476, which completes the coverage of ocp-indent options

@gpetiot
Copy link
Collaborator

gpetiot commented Mar 9, 2022

@hannesm out of curiosity is the compatibility with ocp-indent still a concern today?

@hannesm
Copy link
Author

hannesm commented Mar 9, 2022

@gpetiot not really for me, I'm still using ocp-indent and no ocamlformat. does ocamlformat deal with syntactically invalid sources (i.e. can I just use it instead of ocp-indent)?

Feel free to close this issue, as you find it important or not - I'm fine with ocp-indent and without ocamlformat.

@gpetiot
Copy link
Collaborator

gpetiot commented Mar 9, 2022

The ocamlformat plugin for emacs should be able to deal with invalid files, however it is still not a thing available when invoking dune build @fmt

@hannesm
Copy link
Author

hannesm commented Mar 9, 2022

@gpetiot oh thanks, I'll put trying ocamlformat again with the emacs plugin on my todo list. This is great news! :)

@gpetiot gpetiot closed this as completed Aug 1, 2022
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