Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RHS of => has single cascade parameter prefer to keep it in line #1525

Open
FMorschel opened this issue Aug 15, 2024 · 2 comments
Open

RHS of => has single cascade parameter prefer to keep it in line #1525

FMorschel opened this issue Aug 15, 2024 · 2 comments

Comments

@FMorschel
Copy link

About #1253, when there is a constructor with the last parameter that can "start" at the same line and end at another, this is what we get:

SomeWidget(
  children: [
    long,
    list,
    literal,
  ],
);

SomeWidget(children: [
  for (final o in objects) o,
]);

From the answer to a question I asked at #1523:

The question of how to split within an argument list also has some subtlety [...]. But [...], yes, the formatter does what it calls "block formatting" for the argument list.

I'm not sure how this is handled today and if this will change but with the current formatter, when using cascades from a variable definition and it has only one, it stays in the same line:

final i = 0..abs();
final j = 0
  ..abs()
  ..ceil();

I'd like to request for the new formatter to do the same with the following code and keep the single cascade in the same line if possible.

// Better:
.map((innerMap) => innerMap..addAll({
  'something': someMap['something'],
  //...
}))

// Worse:
.map(
  (innerMap) => innerMap
    ..addAll({
      'something': someMap['something'],
      //...
    }),
)
@munificent
Copy link
Member

There are two separate questions in your example:

  1. When should the formatter split after =>?
  2. When should the formatter split before ..?

When I try the new formatter on your second example, I get:

target.map(
  (innerMap) =>
      innerMap..addAll({
        'something': someMap['something'],
        //...
      }),
);

So, I think as you prefer, it doesn't split before the ... But it does split after the =>. I've tried various rules for allowing not splitting after => even when the body is multiple lines, but it's hard to come up with good ones that don't harm more than they help. The current rules are pretty simple and err on the side of splitting after the =>. That can feel a little spread out than necessary in some cases, but I think the resulting style is at least always clear. Whereas not splitting after => can often lead to code that's too dense and harder to read.

In general, I expect with the new style that code will feel a little more vertically spaced out and less dense than it used to be. (That's why it's called "tall" style, after all.) It's not as compact, but I think it's overall easier to read.

@FMorschel
Copy link
Author

FMorschel commented Aug 17, 2024

I see. It is better to read than before IMO.

I still would prefer that either the RHS of => would not split as you mentioned or that the LHS stays in the first line, but I know it is called "tall style" and this makes more sense for the new formatter.

I guess my main problem with the resulting format is that there are two more indents in the line above the closing parentheses for map. So the inner part feels more "distant" and it seems to be using more space than needed.

@munificent munificent changed the title New tall style: RHS of => has single cascade parameter prefer to keep it in line RHS of => has single cascade parameter prefer to keep it in line Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants