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

Formatting suggestion: if-blocks with multiple conditions #1068

Open
mikez opened this issue Mar 31, 2024 · 4 comments
Open

Formatting suggestion: if-blocks with multiple conditions #1068

mikez opened this issue Mar 31, 2024 · 4 comments

Comments

@mikez
Copy link

mikez commented Mar 31, 2024

Hi @mvdan, I have a formatting suggestion involving if-blocks.

Instead of doing this:

a='aaa'
b='bbb'
c='ccc'
if [[ 
    $a == 'aaa' &&
    $b == 'bbb' &&
    $c == 'ccc' ]] \
    ; then
    echo yes
    # ...
fi

reformat like this:

a='aaa'
b='bbb'
c='ccc'
if [[
    $a == 'aaa'
    && $b == 'bbb'
    && $c == 'ccc'
]]; then
    echo yes
    # ...
fi

The formatting is inspired by what Python does with the black formatter. I think it makes the code more readable. (For operator placement, see also here.)

@ColemanTom
Copy link

I would tend to agree, but recognise others may not. It makes diffs better if you add a new condition, its just a new line being added, rather than a new line being added and the ccc line being changed to shift the closing brackets.

The placement of || or && I think should be governed by the --binary-next-line option perhaps, although that negates my comment about which lines change in a diff.

@mikez
Copy link
Author

mikez commented Sep 6, 2024

@pschmitt it seems you're confused. Want to elaborate? :)

@mvdan
Copy link
Owner

mvdan commented Oct 19, 2024

@ColemanTom refers to the --binary-next-line option, which affects && and || as binary command operators. shfmt puts these at the end of a line by default instead of at the start, to format as:

foo &&
    bar

rather than with --binary-next-line requiring an escaped newline:

foo \
    && bar

Binary operators like && and || in test expressions (or, for that matter, binary expressions in arithmetic expressions) don't seem to have this problem with escaped newlines, because the expression doesn't finish until the closing ]]. So shfmt currently doesn't care nor enforce one style over the other.

You could say that --binary-next-line should enforce the style as follows, and I probably would agree given the current name and description of the flag:

[[
    foo
    && bar
]]

But then, should the lack of the flag, i.e. the default behavior, enforce the opposite style?

[[
    foo &&
    bar
]]

This I'm less convinced about - while consistency is good in formatting, we also don't want to fight the user's intent, and both forms seem OK to me. Plus, we haven't enforced anything by default so far, so it could be a surprising change to make.

Overall I'm mildly in favor of this change, but it feels low-priority.

@mikez
Copy link
Author

mikez commented Oct 19, 2024

@mvdan I appreciate you taking the time to look into this. The approach with requiring --binary-next-line seems reasonable to me as well. I especially enjoy the aesthetics of

]]; then

vs

]] \
    ; then

I also understand the low-priority of this. (Hopefully, AI in the future will allow us to make such changes more easily—where most of the work is about discussing policy as opposed to implementation.)

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

No branches or pull requests

3 participants