Incorrect formatting in `case` expression arm

[maxdeviant]

While working on https://github.com/massivefermion/birl/pull/24 I noticed some weird formatting applied by gleam format in Gleam v1.1.

I pointed it out to @giacomocavalieri and he asked me to open an issue here.

Here is a slightly more minimized reproduction (repo):

fn wibble(value: String) {
  case [], [], [] {
    [day_string, time_string], _, _
    | _, [day_string, time_string], _
    | _, _, [day_string, time_string]
    -> Ok(#(day_string, time_string))
    [_], [_], [_] -> Ok(#(value, "00"))
    _, _, _ -> Error(Nil)
  }
}

I would expect something more like this:

fn wibble(value: String) {
  case [], [], [] {
    [day_string, time_string], _, _
    | _, [day_string, time_string], _
    | _, _, [day_string, time_string] ->
      Ok(#(day_string, time_string))
    [_], [_], [_] -> Ok(#(value, "00"))
    _, _, _ -> Error(Nil)
  }
}

Version Info

λ gleam --version
gleam 1.1.0

λ rebar3 --version
rebar 3.23.0 on Erlang/OTP 25 Erts 13.2.2.9

λ erl
Erlang/OTP 26 [erts-14.2.5] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [jit]

Gleam Logs

λ GLEAM_LOG=trace gleam format
DEBUG built glob set; 1 literals, 1 basenames, 2 extensions, 0 prefixes, 0 suffixes, 0 required extensions, 0 regexes
TRACE reading_file path="./test/formatter_issue_2024_05_13_test.gleam"
DEBUG ignoring ./.gitignore: Ignore(IgnoreMatch(Hidden))
DEBUG ignoring ./build: Ignore(IgnoreMatch(Gitignore(Glob { from: Some("./.gitignore"), original: "/build", actual: "build", is_whitelist: false, is_only_dir: false })))
TRACE reading_file path="./src/formatter_issue_2024_05_13.gleam"
 INFO Successfully completed

[lpil]

Thanks

[Acepie]

It looks like this was an intentional choice https://github.com/gleam-lang/gleam/blob/main/compiler-core/src/format.rs#L1762-L1775 with the justification being in https://github.com/gleam-lang/gleam/issues/2239

[maxdeviant]

It looks like this was an intentional choice https://github.com/gleam-lang/gleam/blob/main/compiler-core/src/format.rs#L1762-L1775 with the justification being in #2239

The above formatting doesn't seem to align with what is shown in the linked issue.

Note how in my example there is no indentation of the arm, which definitely seems wrong.

[Acepie]

Ahhh my bad, i over focused in the arrow location. I'll try to take a look at this tomorrow night

[giacomocavalieri]

Bad news! The fix actually introduced another bug that resulted in a more common case being formatted badly. So we should reopen this and try and find a different way to format this code examples: the thing that I can't figure out is how to keep the arrow on the same line as the last clause.

Here's some test cases we should keep in mind:

fn test_1() {
  case a, b, c {
    _ignored, _ignored, _ignored
    | _ignored, _ignored, _ignored
    | _ignored, _ignored, _ignored
    -> True
  }
}

fn test_2() {
  case a, b, c {
    _, _, _
    | _, _, _
    | this_is_long_and_is_going_to_split_on_multiple,
      lines,
      and_force_the_arrow_to_break
    -> True
  }
}

fn test_3() {
  case a, b, c {
    _, _, _
    | this_is_long_and_is_going_to_split_on_multiple,
      lines,
      and_force_the_alternative_to_break
    | _, _, _
    -> True
  }
}

For the time being I'll revert this fix so that RC-1.2 doesn't have the bug introduced by this fix