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

Tall style: single argument => with multiple .. is very tall #1603

Open
davidmorgan opened this issue Nov 21, 2024 · 5 comments
Open

Tall style: single argument => with multiple .. is very tall #1603

davidmorgan opened this issue Nov 21, 2024 · 5 comments

Comments

@davidmorgan
Copy link

Short style:

      final value = Value((b) => b
        ..anInt = 3
        ..aString = 'four');

Tall style:

      final value = Value(
        (b) =>
            b
              ..anInt = 3
              ..aString = 'four',
      );

Any ideas please? :)

I'd be open to pushing for a new special case syntax for "lambda that immediately does something to its only argument and never references it again" if that's what it takes ;)

@munificent
Copy link
Member

This is essentially the same issue as #1466. (=> and = are formatted using the same code, and cascades and method chains are mostly formatted using the same code.)

I agree, it doesn't look great. I haven't been able to come up with a fix that doesn't do the wrong thing in some cases. Here, if the cascade target expression was longer than just b, like b.someVeryLong.propertyChain that ended up splitting as well, then the formatter wouldn't do the right thing.

I have some ideas on how to fix this, #1466, and #1465, but I haven't been able to get them working with decent performance yet. It's a surprisingly hard problem.

One option might be to do a hacky fix where we don't split after the => if the target expression is suitably short/simple enough that we're confident that a split won't happen inside it. But I've tried to avoid those sorts of hacks in the new formatter as much as possible.

@davidmorgan
Copy link
Author

Thanks :)

Yes, I can see how piling up hacky fixes could lead to problems ;)

@parren-google
Copy link

For what it's worth, for void Function arguments, rewriting to () {} style is now more compact, and emphasizes that there is no return value. As in:

final value = Value((b) {
  b
    ..anInt = 3
    ..aString = 'four';
});

But the single b now looks a bit lost. So this looks even better to me:

final value = Value((builder) {
  builder
    ..anInt = 3
    ..aString = 'four';
});

I assume we cannot get this in tall style:

final value = Value((b) => b
    ..anInt = 3
    ..aString = 'four');

so with a formatter tweak the best we could get is probably:

final value = Value(
  (b) => b
    ..anInt = 3
    ..aString = 'four'
);

This uses the same vertical space and indentation as the () {} example above. So an automatic rewrite to () {} could be an alternative.

@davidmorgan
Copy link
Author

I don't think () {} is workable, unfortunately, because for a single assignment the arrow will often end up inline with no splits at all:

    print((b) => b..x = 3);

whereas the block never will:

    print((b) {
      b..x = 3;
    });

so the arrow is obviously preferred ... if people start using the block for >= 2 assignments, you'd have to convert between the two when adding/removing an assignment :/

@davidmorgan
Copy link
Author

FWIW, I also don't like renaming b->builder, I've seen a few codebases that did that and it wasn't convincing :) ... as it doesn't really add anything.

In fact it would be great if there was a way in the language to not mention it at all:

final value = Value(
  ..anInt = 3
  ..aString = 'four'
);

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

3 participants