ocamlformat icon indicating copy to clipboard operation
ocamlformat copied to clipboard

Feature request: avoid extra indentation for `functor` keyword

Open craigfe opened this issue 4 years ago • 2 comments

Is your feature request related to a problem? Please describe.

Given the following module definition:

module Foo (X : S) = struct
  let v = 1
end

Annotating the type of Foo requires using the functor keyword:

module Foo : MAKER =
functor
  (X : S)
  ->
  struct
    let v = 1
  end

Unfortunately, OCamlformat's style choices here require indenting everything inside the struct, which is potentially hundreds of lines. It seems worthwhile to change the styles to avoid this.

Describe the solution you'd like

The simplest change is probably to be consistent with the wrapped non-functor case and keep struct in line with the module keyword:

module Foo : MAKER =
  functor
    (X : S)
  ->
struct
  let v = 1
end

(* Wrapped non-functor case *)
module Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
    (X : S) =
struct
  let v = 1
end

For what it's worth, I think it's fine to keep this on one line until it needs to wrap:

module Make : MAKER = functor (X : S) -> struct
  let v = 1
end

module Make : MAKER = functor (X : S) (Y : S) -> struct
  let v = 1
end

This makes the refactor a 1-line diff in most cases. I'm used to OCamlformat expanding to a more verbose / conservative style once the wrap-point is reached, so this seems quite natural to me.

craigfe avatar Mar 16 '20 15:03 craigfe

This seems like a good idea to me. I think calling it "refactor-friendly" is a tangent: this proposal reduces indentation in a way that does not cause ambiguity, so good.

For symmetry with the output of the sugared case, it would probably be better to place the -> at the end of the preceding line if possible, analogously to how the = is not placed on its own line.

jberdine avatar May 04 '20 21:05 jberdine

I think calling it "refactor-friendly" is a tangent: this proposal reduces indentation in a way that does not cause ambiguity, so good.

Personally, I don't particularly mind the extra indentation in a vacuum. I do mind that adding / removing a functor constraint may cause a huge indentation diff one way or another. Regardless, I'll change the title accordingly.

craigfe avatar May 05 '20 07:05 craigfe