Weird indentation and splitting when comment appears before cascade setter #1429
Comments
munificent
added a commit
that referenced
this issue
Sep 4, 2024
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
added a commit
that referenced
this issue
Sep 4, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Given:
The new formatter outputs:
There's no reason to indent or split the setter. Curiously, it doesn't seem to have that problem for cascaded methods. This formatting is preserved:
The text was updated successfully, but these errors were encountered: