pnc formatting records · ideas · Zulip Chat Archive

Stream: ideas

Topic: pnc formatting records

Luke Boswell (Jan 08 2025 at 23:45):

Just wanting to register my thought while looking at this. But I wish the formatter didn't force newlines in here.

# from this
Ok({
    status: code,
    headers: [],
    body: Str.to_utf8(body),
})

# to this
Ok(
    {
        status: code,
        headers: [],
        body: Str.to_utf8(body),
    },
)

Sam Mohr (Jan 08 2025 at 23:45):

I made a similar comment in the PR

Sam Mohr (Jan 08 2025 at 23:46):

We should try to fix this before the breaking changes all go through

Luke Boswell (Jan 08 2025 at 23:47):

Another one..

# from this
|> Result.map_err(\err ->
    when err is
        FileReadErr(path, file_err) ->
            Exit(
                -1,
                "Failed to launch server!\nError reading file $(Path.display(path)):\n\t$(Inspect.to_str(file_err))",
            )

        FileReadUtf8Err(path, _) ->
            Exit(
                -2,
                "Failed to launch server!\nError: failed to read file $(Path.display(path)) as utf8.",
            ),
)

# to this
|> Result.map_err(
    \err ->
        when err is
            FileReadErr(path, file_err) ->
                Exit(
                    -1,
                    "Failed to launch server!\nError reading file $(Path.display(path)):\n\t$(Inspect.to_str(file_err))",
                )

            FileReadUtf8Err(path, _) ->
                Exit(
                    -2,
                    "Failed to launch server!\nError: failed to read file $(Path.display(path)) as utf8.",
                ),
)

Luke Boswell (Jan 08 2025 at 23:52):

And another

# from this
stmt =
    Sqlite.prepare!({
        path: db_path,
        query: "SELECT id, task FROM todos WHERE status = :status;",
    })
    |> Result.map_err(\err -> ServerErr("Failed to prepare Sqlite statement: $(Inspect.to_str(err))"))
    |> try

# to this
stmt =
    Sqlite.prepare!(
        {
            path: db_path,
            query: "SELECT id, task FROM todos WHERE status = :status;",
        },
    )
    |> Result.map_err(\err -> ServerErr("Failed to prepare Sqlite statement: $(Inspect.to_str(err))"))
    |> try

Luke Boswell (Jan 08 2025 at 23:52):

I don't mean to be nitpicky... just documenting these as I see them

Sam Mohr (Jan 08 2025 at 23:54):

I presume we should be able to fix this by saying anything in a PNC apply node that only has one arg, and that arg is a closure or a record, we allow it to stay on the same lines as the PNC braces

Sam Mohr (Jan 08 2025 at 23:54):

I don't think we need any other examples unless they can't be fixed by this rule

Anthony Bullard (Jan 09 2025 at 00:39):

This is the standard style for all collections

Anthony Bullard (Jan 09 2025 at 00:39):

And PNC args are in a collection

Sam Mohr (Jan 09 2025 at 00:39):

I think the standard style is Bad

Sam Mohr (Jan 09 2025 at 00:39):

And we should change it

Anthony Bullard (Jan 09 2025 at 00:40):

It is for args

Sam Mohr (Jan 09 2025 at 00:40):

(when it's for a single item)

Anthony Bullard (Jan 09 2025 at 00:40):

And I’m not disagreeing

Anthony Bullard (Jan 09 2025 at 00:40):

Just context setting

Anthony Bullard (Jan 09 2025 at 00:40):

It’ll be a small change

Anthony Bullard (Jan 09 2025 at 00:40):

I might be able to do it tonight

Anthony Bullard (Jan 09 2025 at 00:40):

But in the morning for sure

Luke Boswell (Jan 09 2025 at 00:41):

Definitely not urgent

Sam Mohr (Jan 09 2025 at 00:41):

Yep, not urgent!

Anthony Bullard (Jan 09 2025 at 01:03):

I feel so much pressure

Sam Mohr (Jan 09 2025 at 01:04):

"More weight."

Anthony Bullard (Jan 09 2025 at 01:04):

To be clear

Anthony Bullard (Jan 09 2025 at 01:05):

We are saying that the style for PNC that the trailing paren is always tucked up tight?

Anthony Bullard (Jan 09 2025 at 01:05):

Or only with a single multiline arg?

Sam Mohr (Jan 09 2025 at 01:06):

with a single multiline arg IMO

Sam Mohr (Jan 09 2025 at 01:07):

Oh, actually

Sam Mohr (Jan 09 2025 at 01:10):

My definition would be that:

if there is only one argument, even if it's multiline, the opening parens should touch the start of the first argument
if any of the args are multiline, the closing parens should come on a new line with indentation at the start of the function name up on the first line
- unless it's a record and there's only one arg, then put that last parens at the end of the record (and maybe a tuple?)

Anthony Bullard (Jan 09 2025 at 02:50):

I think your second rule is “any collection that is the sole argument should have no new lines between its braces and the parens of the apply

Anthony Bullard (Jan 09 2025 at 02:51):

But other multiline expressions should (another apply, when, if, etc)

Anthony Bullard (Jan 09 2025 at 02:51):

When I’m done fixing my moms computer and I can use mine again I’ll type out examples

Richard Feldman (Jan 09 2025 at 02:52):

beeeeeeen there

Anthony Bullard (Jan 09 2025 at 02:53):

Richard Feldman said:

beeeeeeen there

Never buy a new SSD for your moms computer for Christmas

Anthony Bullard (Jan 09 2025 at 02:54):

Or pay GeekSquad to install it

Sam Mohr (Jan 09 2025 at 03:47):

Those rules make sense

Joshua Warner (Jan 09 2025 at 05:33):

There's the concept floating around of things being "outdentable", and I think we should try to follow that

Joshua Warner (Jan 09 2025 at 05:34):

That's what already drives formatting of things like this:

foo = {
  1,
}

rather than:

foo =
    {
        1,
    }

Joshua Warner (Jan 09 2025 at 05:35):

That also applies to closures

Joshua Warner (Jan 09 2025 at 05:35):

(and, IIRC, when, etc etc)

Joshua Warner (Jan 09 2025 at 05:36):

But for example, it doesn't apply to if, since that looks weird with the outdented "else"

Joshua Warner (Jan 09 2025 at 05:38):

So the rule I'd suggest here is:

If the collection has exactly one member,
and, that member is multi-line
and, that member is "outdentable"

then we do the formatting suggested here

Joshua Warner (Jan 09 2025 at 05:38):

I'm considering outdentable is somewhat flexible here

Joshua Warner (Jan 09 2025 at 05:39):

In particular, I think this is reasonable to do with single-element lists as well

Sam Mohr (Jan 09 2025 at 09:48):

That makes sense! I don't know how that applies to lambdas, though. I feel like having the closing parens on a new line:

item_list.map(|item|
    first = 1 + 2
    second = item * 3

    first * second
)

Is better than putting the parens on the "outdentable" function body:

item_list.map(|item|
    first = 1 + 2
    second = item * 3

    first * second)

Anthony Bullard (Jan 09 2025 at 11:28):

Seems so special-casey

Joshua Warner (Jan 09 2025 at 12:56):

One way to think about this would be the additional requirement that the last line of the inner thing must be “short”. A closing brace is short. A line from a lambda or a when expr is not in general short, so we’d define it as such.

Anthony Bullard (Jan 09 2025 at 14:59):

I think when using PNC apply (in Patterns or Exprs) you would like the following:

When there is a single arg and it is a single-line collection with no space after stay inline with the apply

foo({ a: "Something", b: "Else", c: "Here" }) # Stays the same

When there is a single arg and it is a multi-line collection the start of the collection touches the open paren, and then end touches the closing paren:

foo({
    a: "Something",
    b: "Else",
     c: "Here"
}) # Original

foo({
    a: "Something",
    b: "Else",
    c: "Here",
}) # Formatted

When there are multiple args, all but the last arg are single-line and the last arg is a multi-line collection the start of the collection stays on the same line as the open paren, and then end touches the closing paren:

foo(bar, baz, {
    a: "Something",
    b: "Else",
     c: "Here"
}) # Original

foo(bar, baz, {
    a: "Something",
    b: "Else",
    c: "Here",
}) # Formatted

When there is a single arg and that arg is a multi-line lambda the lambda header touches the open parens, the function body is indented, and the closing parens goes to a newline

some_fn(\a, b ->
    a(b)) # Original

some_fn(\a, b ->
    a(b),
) # Formatted

When there are multiple args, all but the last arg are single-line and the last arg is a multi-line lambda the lambda header stays on the same the open parens, the function body is indented, and the closing parens goes to a newline

some_fn(foo, bar, \a, b ->
    a(b)) # Original

some_fn(foo, bar, \a, b ->
    a(b),
) # Formatted

I think the question is what is the handling of when and if? Do we prefer this:

some_fn(foo, bar, when baz is
        Ok a -> a
        Err _ -> "something else") # Original

some_fn(foo, bar,
    when baz is
        Ok a -> a
        Err _ -> "something else",
) # Formatted

some_fn(foo, bar, if baz then
        "this"
    else
        "something else") # Original

some_fn(foo, bar,
    if baz then
        "this"
    else
        "something else",
) # Formatted

Or would you rather have the when on the same line here? I'm assuming the if..then has to be forced on to a new line and outdent the whole thing.

Sam Mohr (Jan 09 2025 at 18:02):

Forcing when and if on a new line is good!

Anthony Bullard (Jan 09 2025 at 19:30):

Trying to decide if when and if should make the whole thing multiline and outdented

Sam Mohr (Jan 09 2025 at 19:38):

That's my inclination

Sam Mohr (Jan 09 2025 at 19:40):

Based on what Josh said above, I think if and when aren't "short"

Sam Mohr (Jan 09 2025 at 19:42):

And in addition, they have a target expression they match over that gives context

Sam Mohr (Jan 09 2025 at 19:42):

And it's nice to have that closer to their bodies

Sam Mohr (Jan 09 2025 at 19:42):

Rather than on the same line as the called function, 4 tab widths to the right

Anthony Bullard (Jan 09 2025 at 22:12):

Ok, we just have to make sure we define “short”. Like is a nested apply “short”?

Sam Mohr (Jan 09 2025 at 22:12):

Yep, what is "short"

Sam Mohr (Jan 09 2025 at 22:13):

By nested apply do you mean func.call(second_func.call(a, b, c))?

Anthony Bullard (Jan 10 2025 at 02:06):

Yep

Sam Mohr (Jan 10 2025 at 02:08):

I think "short" means "this thing has indentation at its end at the same level as its start"

Sam Mohr (Jan 10 2025 at 02:08):

E.g.

func.call({
    foo: 1,
    bar: 2,
})

Sam Mohr (Jan 10 2025 at 02:09):

Since the } is where func starts its indentation, even though it "outdented" in the record fields

Anthony Bullard (Jan 10 2025 at 02:09):

So that would include PNC applies

Anthony Bullard (Jan 10 2025 at 02:10):

So all collections and PNC applies(which are an expr and an arg collection)

Sam Mohr (Jan 10 2025 at 02:10):

Where "outdented" means "the middle of this this has indentation"

Anthony Bullard (Jan 10 2025 at 02:10):

I guess as long as the func itself is singleline

Anthony Bullard (Jan 10 2025 at 02:10):

Ok, that's easy enough logic to implement

Sam Mohr (Jan 10 2025 at 02:11):

Yep, if you do

func.call(arg1, arg2,
    |foo| foo + 123)

Anthony Bullard (Jan 10 2025 at 02:11):

I'll get this banged out and then time for new lambda syntax

Sam Mohr (Jan 10 2025 at 02:11):

I'd expect that to format to

func.call(
    arg1,
    arg2,
    |foo| foo + 123,
)

Anthony Bullard (Jan 10 2025 at 02:12):

Well, I meant something weird like:

func.call(arg1, arg2, (if blah then
  func_one
else
  func_two)(some_other_arg))

Anthony Bullard (Jan 10 2025 at 02:12):

Which would format to this I assume:

func.call(
    arg1,
    arg2,
    (if blah then
        func_one
    else
        func_two)(some_other_arg),
)

Sam Mohr (Jan 10 2025 at 02:13):

Oh lol, yeah

Anthony Bullard (Jan 10 2025 at 02:13):

And also, it prints a "YOU'RE FIRED" message

Sam Mohr (Jan 10 2025 at 02:13):

INTO THE SUN"

Anthony Bullard (Jan 10 2025 at 02:13):

Perfecto!

Last updated: Jun 16 2026 at 16:19 UTC