Nested "do" closures are formatted badly

[SimonDanisch]

trafficstars

Just got this result:

That's probably not what we want ;)

[SimonDanisch]

I guess this is a result from trying to apply the YASGuide argument rules for closures...

[domluna]

yeah this is kinda tricky since the indent, in this case, is based on YASGuide.

[nickrobinson251]

I was curious what BlueStyle would do here (sharing in case anyone else is interested)

julia> println(format_text(str, YASStyle()))
positions = Buffer(lift(px, py, pz) do x, y, z
                       return vec(map(CartesianIdices(z)) do i
                                      return GeometryBasics.Point{3,Float32}(get_dim(x, i,
                                                                                     1,
                                                                                     size(z)),
                                                                             get_dim(y, i,
                                                                                     2,
                                                                                     size(z)),
                                                                             z[i])
                                  end)
                   end)

julia> println(format_text(str, BlueStyle()))
positions = Buffer(
    lift(px, py, pz) do x, y, z
        return vec(
            map(CartesianIdices(z)) do i
                return GeometryBasics.Point{3,Float32}(
                    get_dim(x, i, 1, size(z)), get_dim(y, i, 2, size(z)), z[i]
                )
            end,
        )
    end,
)

using JuliaFormatter v0.10

[domluna]

positions = Buffer(lift(px, py, pz) do x, y, z
    return vec(map(CartesianIdices(z)) do i
        return GeometryBasics.Point{3,Float32}(get_dim(x, i, 1, size(z)),
    						       get_dim(y, i, 2, size(z)), z[i])
    end)
end)

Is there a reason why we would do to format the way it currently does with YAS? I think the above would be more favorable but it goes against current YAS style guide afaik

cc @SimonDanisch

[SimonDanisch]

I think this is basically a bug in the YAS style, which falls apart when you apply it as strictly as that^^ To be honest, I really don't like that YAS produces so much wasted white space on the left side in general.

cc @jrevels @ararslan

[jrevels]

I'd write this as the following:

positions = Buffer(lift(px, py, pz) do x, y, z
    return vec(map(CartesianIdices(z)) do i
        return GeometryBasics.Point{3,Float32}(get_dim(x, i, 1, size(z)),
                                               get_dim(y, i, 2, size(z)),
                                               z[i])
    end)
end)

AFAICT this is a result of JuliaFormatter overindenting the inner blocks in attempt to conform to this YASGuide point:

If an argument list extends to multiple lines, align new lines to the parenthesis/bracket/etc. that started the argument list.

Should we explicitly add a line to explicitly only single-indent nested keyword...end blocks? e.g. change

keyword...end statements form "code blocks" if the delimiting terms (keyword and end) are on separate lines. The contents of such code blocks should be indented.

to

keyword...end statements form "code blocks" if the delimiting terms (keyword and end) are on separate lines. The contents of such code blocks should be single-indented w.r.t. its parent block

[jrevels]

keyword...end statements form "code blocks" if the delimiting terms (keyword and end) are on separate lines. The contents of such code blocks should be single-indented w.r.t. its parent block

Oof, after playing with it a bit, this proposal is a bad one if you have multiple blocks nested within an argument list...

current behavior (which I like):

variable = call(map(x) do a, b
                    ⋮ 
                end, 
                map(y) do c, d
                    ⋮
                end)

with proposed change, I guess it'd turn into

variable = call(map(x) do a, b
    ⋮ 
end, map(y) do c, d
    ⋮
end)

Maybe not the most satisfying, but one answer here might be to keep YASGuide the way it is. Then, if overindentation like this gets hairy, it implies you should do a broader code shuffle to make keep things readable 😁

e.g.

λ = lift(px, py, pz) do x, y, z
    transform = i -> GeometryBasics.Point{3,Float32}(get_dim(x, i, 1, size(z)),
                                                     get_dim(y, i, 2, size(z)),
                                                     z[i])
    return vec(map(transform, CartesianIdices(z))) 
end
positions = Buffer(λ)

which I find more readable than any of the above

[domluna]

positions = Buffer(lift(px, py, pz) do x, y, z
    return vec(map(CartesianIdices(z)) do i
        return GeometryBasics.Point{3,Float32}(get_dim(x, i, 1, size(z)),
                                               get_dim(y, i, 2, size(z)),
                                               z[i])
    end)
end)

is what I meant, just now realizing I didn't look at the parenthesis correctly.

That's a good point though, if code formats weirdly often you could take it as a sign it could be written nicer.