Skip to content

Commit

Permalink
Add test that tickles subtree merging. (#1488)
Browse files Browse the repository at this point in the history
An important optimization in the formatter is that it will format a
subtree of the Piece tree separately and weave the result back into the
parent Solution when possible. Part of that process is adding in any
bound states, overflow characters, and costs determined in the subtree.

Surprisingly, those rarely actually come into play in terms of affecting
the outermost winning solution. I'm not sure exactly why, but if you
just merge in the subtree solution's text and discard the bound states,
overflow, and cost... all the tests still pass.

But after testing on a large corpus, it turns out that in more complex
real-world examples, it *is* important to copy that data back over. So
I grabbed an example whose formatting was affected and added this as a
sort of regression test.
  • Loading branch information
munificent committed May 16, 2024
1 parent 93d2068 commit ce04d38
Showing 1 changed file with 38 additions and 0 deletions.
38 changes: 38 additions & 0 deletions test/tall/regression/other/flutter.unit
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,42 @@ main() {
..style.width = '100%'
..style.height = '100%'
..classList.add(_kClassName);
}
>>>
### When a subtree is formatted separately, the mergeSubtree() step mostly has
### no effect because the subtree's cost and overflow doesn't affect the
### winning solution. But it sometimes does. None of the existing tests happen
### to hit a case that does, but when formatting a corpus with subtree merging
### on and off, this was a relatively simple example where the behavior differs.
class C {
void paint(PaintingContext context, Offset offset) {
final (double visualPosition, Color leftColor, Color rightColor) =
switch (textDirection) {
TextDirection.rtl => (
1.0 - _position.value,
_activeColor,
trackColor,
),
TextDirection.ltr => (_position.value, trackColor, _activeColor),
};

final double trackCenter = offset.dy + size.height / 2.0;
final double trackLeft = offset.dx + _trackLeft;
}
}
<<<
class C {
void paint(PaintingContext context, Offset offset) {
final (
double visualPosition,
Color leftColor,
Color rightColor,
) = switch (textDirection) {
TextDirection.rtl => (1.0 - _position.value, _activeColor, trackColor),
TextDirection.ltr => (_position.value, trackColor, _activeColor),
};

final double trackCenter = offset.dy + size.height / 2.0;
final double trackLeft = offset.dx + _trackLeft;
}
}

0 comments on commit ce04d38

Please sign in to comment.