=> closures formatting in named argument list

[goderbauer]

The new dart formatter wants to format this as follows:

void _showDialog(Widget child, BuildContext context) {
  showCupertinoModalPopup<void>(
    context: context,
    builder:
        (BuildContext context) => Container(
          height: 216,
          padding: const EdgeInsets.only(top: 6.0),
          // The Bottom margin is provided to align the popup above the system
          // navigation bar.
          margin: EdgeInsets.only(
            bottom: MediaQuery.of(context).viewInsets.bottom,
          ),
          // Provide a background color for the popup.
          color: CupertinoColors.systemBackground.resolveFrom(context),
          // Use a SafeArea widget to avoid system overlaps.
          child: SafeArea(top: false, child: child),
        ),
  );
}

This gives the closure a lot of prominence over the other parameters and also causes more indentation reducing the effective line length for code inside the closure.

void _showDialog(Widget child, BuildContext context) {
  showCupertinoModalPopup<void>(
    context: context,
    builder: (BuildContext context) => Container(
      height: 216,
      padding: const EdgeInsets.only(top: 6.0),
      // The Bottom margin is provided to align the popup above the system
      // navigation bar.
      margin: EdgeInsets.only(
        bottom: MediaQuery.of(context).viewInsets.bottom,
      ),
      // Provide a background color for the popup.
      color: CupertinoColors.systemBackground.resolveFrom(context),
      // Use a SafeArea widget to avoid system overlaps.
      child: SafeArea(top: false, child: child),
    ),
  );
}

This formatting would visually also be more in line with the formatting I'd expect if builder were a list argument:

void _showDialog(Widget child, BuildContext context) {
  showCupertinoModalPopup<void>(
    context: context, 
    builder: <Widget>[
      longNameForAWidgetToMakeSureThisDoesNotFit,
      longNameForAWidgetToMakeSureThisDoesNotFit2,
      longNameForAWidgetToMakeSureThisDoesNotFit3,
    ],
  );
}

@munificent did mention that there are cases were this kind of formatting for => closures looks worse. I'm interested in seeing examples for that.

[FMorschel]

The same happens in #1525 although it is not a named argument case but a cascade case.

[mernen]

I hadn't found this issue before I opened #1634. I agree, seems like a surprising downgrade when we combine a named argument, a shorthand (=>) closure, and a multi-line expression.

Copying my examples from that issue:

Old formatter:

Widget example1() {
  return ValueListenableBuilder(
    valueListenable: userInfoSource,
    builder: (context, userInfo, _) => ListBody(
      children: [
        Text('Hello ${userInfo.name}'),
        Text('Your email is ${userInfo.email}'),
      ],
    ),
  );
}

Widget example2() {
  return LayoutBuilder(
    builder: (context, constraints) => SingleChildScrollView(
      child: ConstrainedBox(
        constraints: BoxConstraints(minHeight: constraints.maxHeight),
        child: PotentiallyTallWidget(),
      ),
    ),
  );
}

New formatter:

Widget example1() {
  return ValueListenableBuilder(
    valueListenable: userInfoSource,
    builder:
        (context, userInfo, _) => ListBody(
          children: [
            Text('Hello ${userInfo.name}'),
            Text('Your email is ${userInfo.email}'),
          ],
        ),
  );
}

Widget example2() {
  return LayoutBuilder(
    builder:
        (context, constraints) => SingleChildScrollView(
          child: ConstrainedBox(
            constraints: BoxConstraints(minHeight: constraints.maxHeight),
            child: PotentiallyTallWidget(),
          ),
        ),
  );
}

[munificent]

Yeah, I agree this is a corner of the new style that could stand to be tweaked. When first implementing the new style, I put some time into coming up with some heuristics to decide when a closure inside an argument list should be block style and when it shouldn't, but I couldn't find a set of rules that I felt worked well across a large corpus. So the current implementation just doesn't support it and does the simple thing.

But if I can find some time and after I fix some other higher-priority style issues (#1465, #1466), I'd like to revisit this.

[FMorschel]

I think this and the above two issues you mentioned are similar to what I asked at #1551, could that be reconsidered if this is fixed?