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

Weird indentation in chained lambda functions #1175

Closed
a9udn9u opened this issue Feb 12, 2023 · 3 comments
Closed

Weird indentation in chained lambda functions #1175

a9udn9u opened this issue Feb 12, 2023 · 3 comments

Comments

@a9udn9u
Copy link

a9udn9u commented Feb 12, 2023

Actual:

    return maps
        .where((map) => map != null)
        .expand((map) => map!.entries)
        .fold({}, (merged, entry) {
      merged[kp(entry.key)] = vp(entry.value);  // <- Should indent more
      return merged;                            // <- Should indent more
    });                                         // <- Should indent more

Expected:

    return maps
        .where((map) => map != null)
        .expand((map) => map!.entries)
        .fold({}, (merged, entry) {
          merged[kp(entry.key)] = vp(entry.value);
          return merged;
        });

Similarly in function parameter list:

  static List<V> spaceBetween<V>(
    List<V> collection,
    V spacer, {
    bool? bothEnds = false,  // <- Should indent more
    bool? leftEnd = false,   // <- Should indent more
    bool? rightEnd = false,  // <- Should indent more
  }) {                       // <- Should indent more
@munificent
Copy link
Member

    return maps
        .where((map) => map != null)
        .expand((map) => map!.entries)
        .fold({}, (merged, entry) {
      merged[kp(entry.key)] = vp(entry.value);  // <- Should indent more
      return merged;                            // <- Should indent more
    });                                         // <- Should indent more

This formatting is a little weird looking here but is deliberate. It makes more sense when the method chain doesn't itself need to split, as in:

    return maps.where((map) => map != null) {
      merged[kp(entry.key)] = vp(entry.value);
      return merged;                          
    });                                       

Here, the block-like formatting works well and most users seem to like it. It also, I think, works well in code like:

    return maps.where((map) {
      return map != null;
    }).expand((map) {
      return map!.entries;
    }).fold({}, (merged, entry) {
      merged[kp(entry.key)] = vp(entry.value);
      return merged;                          
    });                                       

The formatter does that block-like formatting consistently even if the preceding method chain splits. We've considered whether we should change the closure's indentation based on whether the preceding method chain splits, but it's not clear if that's what users want. It's a big enough change that it would have to go through the change process.

  static List<V> spaceBetween<V>(
    List<V> collection,
    V spacer, {
    bool? bothEnds = false, 
    bool? leftEnd = false,  
    bool? rightEnd = false, 
  }) {                      

This is the deliberate style. It may not be to everyone's taste, but as far as I can tell, most users are OK with it.

@zedefi
Copy link

zedefi commented Apr 24, 2023

Same issue here. A code formatted this way is so hard to read. Please at least provide some way how to customize this, because it is really distracting. Especially the async code with streams that is so difficult to read like this :(

@munificent
Copy link
Member

I ran these examples through the forthcoming tall style and it produces:

    return maps
        .where((map) => map != null)
        .expand((map) => map!.entries)
        .fold({}, (merged, entry) {
          merged[kp(entry.key)] = vp(entry.value);
          return merged;
        });

So that's exactly as expected.

And:

static List<V> spaceBetween<V>(
  List<V> collection,
  V spacer, {
  bool? bothEnds = false,
  bool? leftEnd = false,
  bool? rightEnd = false,
}) {

As I mentioned in my previous comment, that's the expected style. So I think this is working well in the new formatter.

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