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

BinaryExpression Grouping #37

Closed
belav opened this issue Mar 15, 2021 · 7 comments · Fixed by #354
Closed

BinaryExpression Grouping #37

belav opened this issue Mar 15, 2021 · 7 comments · Fixed by #354
Assignees
Labels
area:formatting type:bug Something isn't working
Milestone

Comments

@belav
Copy link
Owner

belav commented Mar 15, 2021

Take this example

var longNameForcesLineBreakYeahLongName =
    someValue == someOtherValue && x == 7;

The && is the root BinaryExpression, which is a LogicalAnd
The left and right of it are EqualsExpression.
CSharpier should group the LogicalAnd, but if it groups every binary Expression than the following doesn't format correctly.

if (
    longStatementName
    && longerStatementName
    && evenLongerStatementName
    && superLongStatementName
) { }

Which is basically a tree of BinaryExpressions where the Left is LogicalAnd BinaryExpression and the Right is IdentifierName.
This may require flattening out BinaryExpressions and conditionally grouping them based on what operation they are performing.

See prettier/src/language-js/print/binaryish.js for all the logic that goes into making this work for js

@belav belav added type:bug Something isn't working area:formatting labels Mar 15, 2021
@belav belav added this to the Beta milestone Mar 15, 2021
@belav
Copy link
Owner Author

belav commented Apr 10, 2021

This is also related

    return value == null
    || string.IsNullOrEmpty(value.Trim());
// should be
    return value == null
        || string.IsNullOrEmpty(value.Trim());

@belav
Copy link
Owner Author

belav commented Apr 12, 2021

This was partially addressed in #84 but requires a lot more work

@belav
Copy link
Owner Author

belav commented Apr 12, 2021

this looks weird

        if (
            trivia.Kind() == SyntaxKind.SingleLineCommentTrivia
            || trivia.Kind()
            == SyntaxKind.SingleLineDocumentationCommentTrivia1111111
        ) { }

@belav
Copy link
Owner Author

belav commented May 5, 2021

See also

        protected override bool IsBinaryLike(ExpressionSyntax node) =>
            node is BinaryExpressionSyntax
            || node
                is IsPatternExpressionSyntax isPattern
            && isPattern.Pattern is ConstantPatternSyntax;

@dishuk13
Copy link
Contributor

Related

while (
  // stop if we have reached the end of the token being read
  initialDepth - 1 <
    reader.Depth - (JsonTokenUtils.IsEndToken(reader.TokenType) ? 1 : 0) &&
  writeChildren &&
  reader.Read()
);

https://github.com/belav/Newtonsoft.Json/blob/397e45649325788b5ea299be3ca5303b86eeaf9b/Src/Newtonsoft.Json/JsonWriter.cs#L773

@belav belav removed this from the Beta milestone Jun 14, 2021
@respel
Copy link

respel commented Jun 16, 2021

See also

if (
    query
    && (currentChar == '='
    || currentChar == '<'
    || currentChar == '!'
    || currentChar == '>'
    || currentChar == '|'
    || currentChar == '&')
) {
    ended = true;
}

The current alignment leads you to believe that the && and || operators are chained but they aren't really. We should do some kind of nesting here.

@belav
Copy link
Owner Author

belav commented Jul 12, 2021

Make sure to handle the test case in SwitchExpression that isn't indenting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:formatting type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants