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

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

Open
NightBlues opened this issue Mar 1, 2024 · 1 comment
Open

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

NightBlues opened this issue Mar 1, 2024 · 1 comment

Comments

@NightBlues
Copy link

NightBlues commented Mar 1, 2024

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
  1. Create test.ml file:
let f x =
  match [] with
  | [] ->
    (match x with
     | 1 -> ()
     | _ -> ());
    ()
  | _ -> ()
  1. Run ocp-indent to check that indentation left untouched.
  2. Create file .ocamlformat
profile = default
break-cases = fit-or-vertical
cases-exp-indent = 2
ocp-indent-compat = true
  1. 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
  1. 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 -> ()
    | _ -> ());
    ()
  | _ -> ()
$ 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
Copy link
Collaborator

gpetiot commented Apr 25, 2024

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)

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

2 participants