New Formatter not formatting nicely after comments

[atreeon]

What input code did you provide to the formatter?

    final isValid = //
        context.select((ProjectEditBloc bloc) => (bloc.state as ProjectEditState_data).isValid);

What output did the formatter produce?

    final isValid = //
    context.select(
      (ProjectEditBloc bloc) => (bloc.state as ProjectEditState_data).isValid,
    );

What output did you expect or want the formatter to produce?
I have used comments to force a line break in order to improve readability. This has worked really well since almost day one of Flutter.

After a comment line, the next line should be indented.

Also, the comma has been added, even though I haven't gone over my max chars per line setting.

formatter:
  trailing_commas: preserve
  page_width: 500

    final isValid = //
        context.select((ProjectEditBloc bloc) => (bloc.state as ProjectEditState_data).isValid);

Anything else we should know?

Thanks!

[munificent]

Yeah, the formatter can get a little confused if comments appear in uncommon locations. This is one that could definitely be improved. Thanks for filing the issue.

[atreeon]

another odd one one here, many thanks!

The empty string should be on the same line as the null operator ??. Or (my less favourable outcome) it should be in line with the other methods.

    return snap //
            .docChanges
            .map((change) => change.doc.data()?['sessionId'] as String?)
            .whereNotNull()
            .toSet()
            .firstOrNull ??
        '';

[munificent]

That one is working as intended. The formatter generally splits infix operators if there is a newline in either operand and then indents the operand further. This is to make it clear what the operator precedence is and thus which parts of the expression are the operand and which are outside of the operator. In your example, that entire method chain is the left operand to ??, so it splits at the ?? and indents the method chain further to emphasize that.

I admit it does look kind of funny sometimes. That nesting structure would be more obvious if we split before infix operators instead of after:

    return snap //
            .docChanges
            .map((change) => change.doc.data()?['sessionId'] as String?)
            .whereNotNull()
            .toSet()
            .firstOrNull
        ?? '';

But most Dart users seem to prefer them at the ends of lines so that's what the formatter does.

We could try to special case and allow the infix operator to not split in some cases, but it can get weird. Consider how hard to read the code would be if both operands were long method chains:

    return snap 
        .docChanges
        .map((change) => change.doc.data()?['sessionId'] as String?)
        .whereNotNull()
        .toSet()
        .firstOrNull ?? another
        .docChanges
        .map((change) => change.doc.data()?['sessionId'] as String?)
        .whereNotNull()
        .toSet()
        .firstOrNull;

To avoid this kind of output, the formatter has a pretty simple rule for infix operators.

[atreeon]

sure, I guess I don't have a problem with it going onto a new line but it would be nice if it formatted the string inline (as per your example) or indented it one tab to the right (can't put into words why it makes sense to indent it but it looks nicer that way!)

so:

    return snap //
            .docChanges
            .map((change) => change.doc.data()?['sessionId'] as String?)
            .whereNotNull()
            .toSet()
            .firstOrNull ??
            '';

or:

    return snap //
            .docChanges
            .map((change) => change.doc.data()?['sessionId'] as String?)
            .whereNotNull()
            .toSet()
            .firstOrNull ??
              '';

and not:

    return snap //
            .docChanges
            .map((change) => change.doc.data()?['sessionId'] as String?)
            .whereNotNull()
            .toSet()
            .firstOrNull ??
        '';

how does that sound

[atreeon]

I think this one too, the Text widget should be indented as otherwise it looks like it is another case in the switch.

        return switch (_stateProjects) {
          ProjectsDropdownState_loading() => Text("Loading"),
          ProjectsDropdownState_error() || ProjectsDropdownState_notSignedIn() => //
          Text("Error loading lists"),