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

Indentation with chained calls with long arguments is very confusing #1092

Closed
Hixie opened this issue Jan 8, 2022 · 4 comments
Closed

Indentation with chained calls with long arguments is very confusing #1092

Hixie opened this issue Jan 8, 2022 · 4 comments

Comments

@Hixie
Copy link

Hixie commented Jan 8, 2022

I can't figure out what combination of commas to put in this code to make the formatting reasonable:

void assertThatImageLoadingSucceeds(NetworkImageWithRetry subject) {
  subject
      .load(
    subject,
    _ambiguate(PaintingBinding.instance)!.instantiateImageCodec,
  )
      .addListener(
    ImageStreamListener(
      expectAsync2((ImageInfo image, bool synchronousCall) {
	expect(image.image.height, 1);
	expect(image.image.width, 1);
      }),
    ),   
  );  
}
@AlexV525
Copy link

AlexV525 commented Jan 9, 2022

More cases that my friend reported to me 6 months ago, https://github.com/iota9star/mikan_flutter/blob/ae591e1de035f6b845acb11b2e00b634d85b9c2a/lib/internal/http_cache_manager.dart#L46 :

final Completer<File?> completer = Completer();
_httpCacheManager
    ._(
  url,
  cacheKey: cacheKey,
  cacheDir: cacheDir,
  headers: headers,
  chunkEvents: chunkEvents,
  cancelable: cancelable,
)
    .then((value) {
  completer.complete(value);
}).catchError((dynamic error, StackTrace stackTrace) {
  completer.completeError(error, stackTrace);
}).whenComplete(() => _tasks.remove(url));

@munificent
Copy link
Member

Yeah, those are both pretty bad. These are more cases where the formatting rules for argument lists with trailing commas interacts poorly with other formatting rules.

@munificent
Copy link
Member

Wow, yeah. These examples really hit the limitations of the old formatter. I ran them through the forthcoming tall style and got:

void assertThatImageLoadingSucceeds(NetworkImageWithRetry subject) {
  subject
      .load(
        subject,
        _ambiguate(PaintingBinding.instance)!.instantiateImageCodec,
      )
      .addListener(
        ImageStreamListener(
          expectAsync2((ImageInfo image, bool synchronousCall) {
            expect(image.image.height, 1);
            expect(image.image.width, 1);
          }),
        ),
      );
}

And:

_httpCacheManager
    ._(
      url,
      cacheKey: cacheKey,
      cacheDir: cacheDir,
      headers: headers,
      chunkEvents: chunkEvents,
      cancelable: cancelable,
    )
    .then((value) {
      completer.complete(value);
    })
    .catchError((dynamic error, StackTrace stackTrace) {
      completer.completeError(error, stackTrace);
    })
    .whenComplete(() => _tasks.remove(url));

Those look worlds better to me.

@Hixie
Copy link
Author

Hixie commented Aug 2, 2024

That looks so much better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants