ocaml-ppx / ocamlformat

Auto-formatter for OCaml code
MIT License
633 stars 179 forks source link

Bug: match-indent-nested and ocp-indent-config do not work #2531

Open NightBlues opened 8 months ago

NightBlues commented 8 months ago

Describe the bug According to man page, usage of ocp-indent-config: strict_with sets function-indent-nested and match-indent-nested, but it doesn't seem happen - neither ocamlformat --ocp-indent-config --print-config shows changed value, nor behavior changes. This create conflict between ocamlformat and ocp-indent because each of them forces its own indentation.

How to Reproduce Steps to reproduce the behavior:

  1. Create .ocp-indent file (all params are excplicitly set defaults):
    match_clause = 2
    with = 0
    strict_with = never
  2. Create test.ml file:
    let f x =
    match [] with
    | [] ->
    (match x with
     | 1 -> ()
     | _ -> ());
    ()
    | _ -> ()
  3. Run ocp-indent to check that indentation left untouched.
  4. Create file .ocamlformat
    profile = default
    break-cases = fit-or-vertical
    cases-exp-indent = 2
    ocp-indent-compat = true
  5. Run ocamlformat --ocp-indent-config test.ml:
    let f x =
    match [] with
    | [] ->
    (match x with
    | 1 -> ()
    | _ -> ());
    ()
    | _ -> ()

    Note: '|' are under '(', not under letter 'm'.

  6. Add to .ocamlformat:
    match-indent = 0
    match-indent-nested = never
  7. Run ocamlformat --ocp-indent-config test.ml - result is the same, match-indent-nested = never setting is ignored.

Additional info

  1. proof that ocp-indent works: Changing strict_with = always in .ocp-indent file and run ocp-indent again, changes test.ml to:
    let f x =
    match [] with
    | [] ->
    (match x with
    | 1 -> ()
    | _ -> ());
    ()
    | _ -> ()
  2. $ ocamlformat --ocp-indent-config test.ml > /tmp/ocamlformat
    $ ocp-indent test.ml >/tmp/ocp-indent
    $ diff /tmp/ocamlformat /tmp/ocp-indent 
    5,6c5,6
    <     | 1 -> ()
    <     | _ -> ());
    ---
    >      | 1 -> ()
    >      | _ -> ());
gpetiot commented 6 months ago

To give some context, ocamlformat never aimed at subsuming ocp-indent, nor be fully compatible. Because a few of ocp-indent settings don't make sense from ocamlformat's pov, I don't remember which ones, but that's the conclusion I made when I implemented the ocp-indent-compat option back then.

The goal of the ocp-indent-compat option was to make it easier for ocp-indent to run after ocamlformat, as some patterns were easier than others for ocp-indent to process (e.g. if some symbols are at the end of the line vs at the beginning of the line the indentation might differ). This option was necessary for Jane Street projects as they were applying ocp-indent after ocamlformat.

The doc might need to be updated indeed, maybe this option sets too high expectations.

(Imho we might as well remove the feature once this is not useful for Jane Street anymore)