Apply review feedback.

Author: munificent

lib/src/back_end/code.dart

@@ -135,7 +135,7 @@ final class GroupCode extends Code {
 
 /// A [Code] object for a newline followed by any leading indentation.
 final class _NewlineCode extends Code {
-  /// True if a blank line (two newlines) should be written.
+  /// Whether a blank line (two newlines) should be written.
   final bool _blank;
 
   /// The number of spaces of indentation after this newline.

lib/src/back_end/code_writer.dart

@@ -199,6 +199,35 @@ final class CodeWriter {
   }
 
   /// The 3.7 style of indentation and collapsible indentation tracking.
+  ///
+  /// Splits in if-case and for-in loop headers are tricky to indent gracefully.
+  /// For example, if an infix expression inside the case splits, we don't want
+  /// it to be double indented:
+  ///
+  ///     if (object
+  ///         case veryLongConstant
+  ///                 as VeryLongType) {
+  ///       ;
+  ///     }
+  ///
+  /// That suggests that the [IfCasePiece] shouldn't add indentation for the
+  /// case pattern since the [InfixPiece] inside it will already indent the RHS.
+  ///
+  /// But if the case is a variable pattern that splits, the [VariablePiece]
+  /// does *not* add indentation because in most other places where it occurs,
+  /// that's what we want. If the [IfCasePiece] doesn't indent the pattern, you
+  /// get:
+  ///
+  ///     if (object
+  ///         case VeryLongType
+  ///         veryLongVariable
+  ///         ) {
+  ///       ;
+  ///     }
+  ///
+  /// To deal with this, 3.7 had a notion of "collapsible" indentation. In 3.8
+  /// and later, there is a different mechanism for merging indentation kinds.
+  /// This function implements the former.
   void _pushIndentV37(Indent indent, {bool canCollapse = false}) {
     var parentIndent = _indentStack.last.spaces;
     var parentCollapse = _indentStack.last.collapsible;

lib/src/back_end/solution_cache.dart

@@ -23,7 +23,7 @@ import 'solver.dart';
 /// indentation. When that happens, sharing this cache allows us to reuse that
 /// cached subtree Solution.
 final class SolutionCache {
-  /// True if this cache and all solutions in it use the 3.7 style solver.
+  /// Whether this cache and all solutions in it use the 3.7 style solver.
   final bool isVersion37;
 
   final _cache = <_Key, Solution>{};

lib/src/front_end/piece_factory.dart

@@ -115,7 +115,7 @@ mixin PieceFactory {
 
   NodeContext get parentContext;
 
-  /// True if the code being formatted is at language version 3.7.
+  /// Whether the code being formatted is at language version 3.7.
   bool get isVersion37 => formatter.languageVersion == Version(3, 7, 0);
 
   void visitNode(AstNode node, NodeContext context);

lib/src/piece/assign_v37.dart

@@ -6,7 +6,9 @@ import 'piece.dart';
 
 /// A piece for the 3.7 style of an assignment-like construct where an operator
 /// is followed by an expression but where the left side of the operator isn't
-/// also an expression. Used for:
+/// also an expression.
+///
+/// Used for:
 ///
 /// - Assignment (`=`, `+=`, etc.)
 /// - Named arguments (`:`)

lib/src/piece/chain.dart

@@ -48,6 +48,43 @@ import 'piece.dart';
 ///           argument,
 ///           argument,
 ///         );
+///
+/// A challenge with formatting call chains is how they look inside an
+/// assignment, named argument, or `=>`. We don't a newline in a call chain to
+/// force the surrounding assignment to split, because that's unnecessarily
+/// spread out:
+///
+///     variable =
+///         target
+///             .method()
+///             .another();
+///
+/// It's better as:
+///
+///     variable = target
+///         .method()
+///         .another();
+///
+/// But if the target itself splits, then we do want to force the assignment to
+/// split:
+///
+///     // Worse:
+///     variable = (veryLongTarget +
+///             anotherOperand)
+///         .method()
+///         .another();
+///
+///     // Better:
+///     variable =
+///         (veryLongTarget
+///             anotherOperand)
+///         .method()
+///         .another();
+///
+/// The 3.7 formatter had a limited ability to handle code like the above. In
+/// 3.8 and later, [Shape.headline] lets us express the constraint we want here
+/// directly. Since the logic is different, there are two different [ChainPiece]
+/// subclasses for each.
 abstract base class ChainPiece extends Piece {
   /// Allow newlines in the last (or next-to-last) call but nowhere else.
   static const State _blockFormatTrailingCall = State(1, cost: 0);

lib/src/piece/constructor.dart

@@ -52,7 +52,7 @@ final class ConstructorPiece extends Piece {
 
   static const _splitBetweenInitializers = State(2, cost: 2);
 
-  /// True if there are parameters or comments inside the parameter list.
+  /// Whether there are parameters or comments inside the parameter list.
   ///
   /// If so, then we allow splitting the parameter list while leaving the `:`
   /// on the same line as the `)`.