search

OCamlPro / ocp-indent

Indentation tool for OCaml, to be used from editors like Emacs and Vim.
http://www.typerex.org/ocp-indent.html
Other
200 stars 63 forks source link

compile error and allow .fe.sexp at the root #205

Closed mbarbin closed 8 years ago

mbarbin commented 8 years ago

Hello. This pull request contains two unrelated changes, I am happy to split it if relevant. I am surprised by the compile error so I suspect I might just have missed a similar fix already existing somewhere. I am just getting started on ocp-ident @ janestreet so feel free to give feed back about the way you'd like pull request to be presented / organized. Best, Mathieu.

1) gitignore allowing .fe.sexp at the root of the repo 2) Tried to build master today, and got the following error:

  File "src/indentArgs.ml", line 236, characters 62-65:
  Error: This expression has type
           [ `I of string * string
           | `Noblank
           | `P of string
           | `Pre of string
           | `S of string ] list
         but an expression was expected of type Cmdliner.Manpage.block list
         Type
           [ `I of string * string
           | `Noblank
           | `P of string
           | `Pre of string
           | `S of string ]
         is not compatible with type
           Cmdliner.Manpage.block =
             [ `I of string * string | `Noblank | `P of string | `S of string ] 
         The second variant type does not allow tag(s) `Pre
AltGr commented 8 years ago

Hey mon Mathieu! Comment va ?

The build error is due to a change in cmdliner 0.9.8 vs 0.9.7 (we repeated the type so that ocp-indent-lib doesn't depend on cmdliner, but I didn't foresee this issue).

See 30d436ef12f51708195b741100b1ed7c5003067d. Be aware that #194 is expected to get merged soonish if you intend to make changes (it should make things simpler, i.e. easier to understand and to maintain, too).

mbarbin commented 8 years ago

Hey Louis ! Looking forward to working again with you :-)

Turns out I was missing a change related to our build. Getting it fixes the compilation, so I'll close this pull request, and do another one dedicated to just the .fe.sexp change.

Thanks, and sorry for the noise.