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

Avoid gratuitous splits after =. #1554

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Avoid gratuitous splits after =. #1554

merged 1 commit into from
Sep 4, 2024

Conversation

munificent
Copy link
Member

This makes two changes that improve how assignments are formatted:

  1. If a comment is before a cascade setter, don't force the setter to split.

Prior to this change, if you had:

target
  // Comment
  ..setter = value;

The formatter would give you:

target
  // Comment
  ..setter =
      value;

It attached the comment to the ..setter, which was the LHS of the assignment. Then, because there was a newline inside the LHS, it forced a split at the =. Instead, we hoist those leading comments out, similar to how we handle binary operators. In fact, I did some refactoring to get rid of duplicate code in InfixPiece that handled leading comments.

  1. If the LHS of an assignment is a call chain, don't force the assignment to split if the chain does.

In the process of fixing 1, I ran into a similar problem. If you had:

target
    // Comment
    .setter = value;

The formatter would give you:

target
    // Comment
    .setter =
        value;

However, the solution is different in this case. With cascade setters, the target of the = is just the ..setter. With a non-cascade setter, the target is the entire target // Comment \n.setter part. That does contain a newline, and we can't hoist the comment out of the assignment because the target really is the entire target // Comment \n.setter expression.

Instead, we treat call chains on the LHS of assignments as "block formattable". "Block" is kind of a misnomer here because what it really means is "allow a newline without splitting at the = or increasing indentation". I can't think of a good term for that.

That change fixes the above example, but also generally improves how setter calls are formatted when the target is a large call chain expression like:

// Before this PR:
target.method(
      argument,
    ).setter =
    value;

// After:
target.method(argument)
    .setter = value;

This second change isn't entirely... principled? But I tested on a giant corpus and when it kicks in, it invariably produces nicer output.

Fix #1429.

This makes two changes that improve how assignments are formatted:

1. If a comment is before a cascade setter, don't force the setter to split.

Prior to this change, if you had:

```dart
target
  // Comment
  ..setter = value;
```

The formatter would give you:

```dart
target
  // Comment
  ..setter =
      value;
```

It attached the comment to the `..setter`, which was the LHS of the assignment. Then, because there was a newline inside the LHS, it forced a split at the `=`. Instead, we hoist those leading comments out, similar to how we handle binary operators. In fact, I did some refactoring to get rid of duplicate code in InfixPiece that handled leading comments.

2. If the LHS of an assignment is a call chain, don't force the assignment to split if the chain does.

In the process of fixing 1, I ran into a similar problem. If you had:

```dart
target
    // Comment
    .setter = value;
```

The formatter would give you:

```dart
target
    // Comment
    .setter =
        value;
```

However, the solution is different in this case. With cascade setters, the target of the `=` is just the `..setter`. With a non-cascade setter, the target is the entire `target // Comment \n.setter` part. That *does* contain a newline, and we can't hoist the comment out of the assignment because the target really is the entire `target // Comment \n.setter` expression.

Instead, we treat call chains on the LHS of assignments as "block formattable". "Block" is kind of a misnomer here because what it really means is "allow a newline without splitting at the `=` or increasing indentation". I can't think of a good term for that.

That change fixes the above example, but also generally improves how setter calls are formatted when the target is a large call chain expression like:

```dart
// Before this PR:
target.method(
      argument,
    ).setter =
    value;

// After:
target.method(argument)
    .setter = value;
```

This second change isn't entirely... principled? But I tested on a giant corpus and when it kicks in, it invariably produces nicer output.

Fix #1429.
@munificent munificent merged commit 7276774 into main Sep 4, 2024
7 checks passed
@munificent munificent deleted the comments-in-chains branch September 4, 2024 22:31
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.

Weird indentation and splitting when comment appears before cascade setter
2 participants