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

Allow the condition part of a split conditional expression on the same line as an assignment #1465

Open
munificent opened this issue May 6, 2024 · 3 comments

Comments

@munificent
Copy link
Member

In the tall style, we generally split at the operator of an "assignment" (=, :, or =>) if the right hand side splits (and it's block-like). So these are all fine:

// No split:
thing = left + right;

// Block-like split:
thing = [
  element,
];

// Split at operator:
thing =
    veryLongOperand +
    anotherLongOperand;

Currently, that implies that when a conditional expression splits, the entire expression is moved down past the operator, like:

thing = 
    condition
    ? thenBranch
    : elseBranch;

I think it would look nicer if we allowed the condition part to stay on the same line as the assignment in that case:

thing = condition
    ? thenBranch
    : elseBranch;

Note that this would only apply if the condition part of the conditional expressions fits on one line. If the condition itself splits, then we should also split the surrounding assignment. So this wouldn't be allowed:

thing = veryLong ||
        conditionExpression
    ? thenBranch
    : elseBranch;

And you would instead get:

thing =
    veryLong ||
        conditionExpression
    ? thenBranch
    : elseBranch;
@natebosch
Copy link
Member

I think the current tall style might be an improvement. The case in df97775 looks better with the first operand on the next line because it makes the indentation against that operand more obvious.

Currently, that implies that when a conditional expression splits, the entire expression is moved down past the operator, like:

Is your example correct? I though the condition operand would have less indentation than the branches while you show them aligned.

@munificent
Copy link
Member Author

munificent commented May 6, 2024

I think the current tall style might be an improvement. The case in df97775 looks better with the first operand on the next line because it makes the indentation against that operand more obvious.

You might be right. I tend to prefer the condition being on the same line as the assignment when it fits, but it's mostly just an aesthetic preference. (For what it's worth, I think the old short style more or less keeps the condition expression on the first line in these cases, though I'm not 100% sure since the rules are quite different.)

Is your example correct? I though the condition operand would have less indentation than the branches while you show them aligned.

I think it looks OK like that, but that's a good question. I could see us going either way. I think we'd have to see what actually works with the surrounding code and is implementable.

@munificent munificent changed the title [tall] Allow the condition part of a split conditional expression on the same line as an assignment Allow the condition part of a split conditional expression on the same line as an assignment Aug 5, 2024
@munificent munificent removed the tall label Aug 6, 2024
@goderbauer
Copy link

I would prefer keeping the condition on the same line. I also find it looks more visually pleasing, particularly in widgets trees.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants