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

zig fmt: Fix formatting of if expressions #21703

Closed
wants to merge 1 commit into from

Conversation

87flowers
Copy link

This fixes some issues with zig fmt and if expressions.

Sometimes when formatting an if expression, the renderer does not carry on with the correct indentation until the end of the if expression. See examples below.

Before PR

test {
    const foo =
        if (1 == 2)
        1
    else if (3 > 4)
        2
    else
        0;

    const foo, const bar =
        if (1 == 2)
        .{ 0, 0 }
    else if (3 > 4)
        .{ 1, 1 }
    else
        .{ 2, 2 };

    const foo = 1 +
        if (1 == 2)
        2
    else
        0;

    errval catch |e|
        if (e == error.Meow)
        return 69
    else
        unreachable;

    return if (1 == 2)
        1
    else if (3 > 4)
        2
    else
        0;
}

After PR

test {
    const foo =
        if (1 == 2)
            1
        else if (3 > 4)
            2
        else
            0;

    const foo, const bar =
        if (1 == 2)
            .{ 0, 0 }
        else if (3 > 4)
            .{ 1, 1 }
        else
            .{ 2, 2 };

    const foo = 1 +
            if (1 == 2)
                2
            else
                0;

    errval catch |e|
        if (e == error.Meow)
            return 69
        else
            unreachable;

    return if (1 == 2)
        1
    else if (3 > 4)
        2
    else
        0;
}

@castholm
Copy link
Contributor

Unfortunately, while this fix solves some erroneous cases, it also introduces new ones:

// before
const foo = 1 + if (1 == 2)
    2
else
    0;

const bar = 100 + calculate(
    200,
    300,
);

// after
const foo = 1 + if (1 == 2)
        2
    else
        0;

const bar = 100 + calculate(
        200,
        300,
    );

I have briefely looked into fixing this issue with zig fmt in the past and I believe the problem with the indentation of if-else expressions goes deeper than what can be easily fixed by adjusting some ais.pushIndent/popIndent calls. A proper fix would likely require a complete rethinking and rewrite of the way indentation is tracked and handled by the renderer.

@87flowers
Copy link
Author

Many thanks, I'll take a deeper look at this and revisit at a later date.

@87flowers 87flowers closed this Oct 15, 2024
@87flowers 87flowers deleted the if-expr-fmt-indent branch October 15, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants