dart-lang / dart_style

An opinionated formatter/linter for Dart code
https://pub.dev/packages/dart_style
BSD 3-Clause "New" or "Revised" License
646 stars 121 forks source link

RHS of commented swicth expression `=>` prefer to keep in line #1551

Closed FMorschel closed 2 months ago

FMorschel commented 2 months ago

Currently, if you have a switch expression with a comment before the => it splits the result into a new line for no good reason:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || // No-break space.
  0xFEFF // Zero width no-break space.
    => 
    1,
  _ => 2,
};

I suggest this new line should only occur when it goes over the line length limit.


More context at: https://github.com/dart-lang/sdk/issues/56597

FMorschel commented 2 months ago

It would seem more similar to the Switch Statement:

switch (value) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  case 0x00A0 || // No-break space.
        0xFEFF: // Zero width no-break space.
    return 1;
  default:
    return 2;
}
munificent commented 2 months ago

I think this one is mostly a consequence of where you're putting the line comment. By putting it between the 0xFEFF and the =>, the formatter considers it part of the pattern. Since that means the pattern then contains a newline (from the line comment itself), it splits at the => (to avoid confusion when a large multiline pattern would otherwise bury the =>).

In the switch statement example, note how the comment is after the :. If you were to move it before the :, you'd probably get some weird formatting there too. The output looks a little better if you move the comment after =>:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || // No-break space.
  0xFEFF => // Zero width no-break space.
    1,
  _ => 2,
};

You could also do:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 /* No-break space. */ || 0xFEFF /* Zero width no-break space. */ => 1,
  _ => 2,
};

If it were my code, since the body is so short, I would probably duplicate it and do:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 => 1, // No-break space.
  0xFEFF => 1, // Zero width no-break space.
  _ => 2,
};

Sometimes the way to the best looking code is to have a little back and forth with the formatter until you get it into a form that it's best optimized for.

Thank you for filing the issue. :)

FMorschel commented 2 months ago

Yes, I know this output happens (below). This is why I filed this issue. I would prefer it to be formatted the way I mentioned in the OP. If you disagree it is alright. I just thing it is weird for the 1, to be alone in any case. If it were a big variable or calculation I would be alright with it, but small expressions like a single number are visually awkward when left alone.

Code I'm mentioning:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || // No-break space.
  0xFEFF => // Zero width no-break space.
    1,
  _ => 2,
};

Take a look at this example in the SDK issue mentioned above (Flutter SDK code replacing the returns):

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || 
  0x1680 ||
  0x180E ||
  0x2000 ||
  0x2001 ||
  0x2002 ||
  0x2003 ||
  0x2004 ||
  0x2005 ||
  0x2006 ||
  0x2007 || // Comments here were deleted by the assist, in the actual code all lines have it
  0x2008 ||
  0x2009 ||
  0x200A ||
  0x200B ||
  0x2028 ||
  0x2029 ||
  0x202F ||
  0x205F ||
  0x3000 ||
  0xFEFF => // Zero width no-break space.
    1,
  _ => 2
};