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 on split variable declarations #1604

Open
iinozemtsev opened this issue Nov 27, 2024 · 3 comments · Fixed by #1612
Open

Indentation on split variable declarations #1604

iinozemtsev opened this issue Nov 27, 2024 · 3 comments · Fixed by #1612

Comments

@iinozemtsev
Copy link
Member

iinozemtsev commented Nov 27, 2024

When the field declaration is long, so that the name has to be wrapped, it used to be wrapped with an indent, like this:

class C {
  final LoremIpsumDolorSitAmetFactory
      _loremIpsumDolorSitAmetFactory;
}

Now there's no indent, and it is harder to read, especially when there are many fields like that.

class C {
  final LoremIpsumDolorSitAmetFactory
  _loremIpsumDolorSitAmetFactory;
}

Also when there's a comment between an annotation and a type, field name is put on a new line:

Before:

  @override
  // ignore: hash_and_equals
  final int hashCode;

Now:

  @override
  // ignore: hash_and_equals
  final int
  hashCode;
@munificent
Copy link
Member

Now there's no indent, and it is harder to read, especially when there are many fields like that.

class C {
  final LoremIpsumDolorSitAmetFactory
  _loremIpsumDolorSitAmetFactory;
}

This is working as intended. I wrote a comment about that here: #1541 (comment).

There's really no perfect solution when you have a really long type annotation followed by a variable or function name. Indenting always looked weird. Not indenting looks kind of weird too, but after trying both styles on a large corpus, I think the latter is generally better. It's consistent with things like metadata annotations and doc comments which also go on preceding lines but don't cause the variable or function name to be indented.

Also when there's a comment between an annotation and a type, field name is put on a new line:

  @override
  // ignore: hash_and_equals
  final int
  hashCode;

This is a bug. I'll take a look.

munificent added a commit that referenced this issue Dec 6, 2024
Comments and metadata before a declaration are always tricky because the formatter by default attaches comments to the innermost piece following the comment. So in:

```dart
// Comment
int field;
```

We want the comment attached to the Piece for the entire field declaration. But the formatter won't see the comment until it hits the `int` token and will end up attaching it to the piece for the type annotation which is embedded inside the field piece. That in turn means that the formatter will think there is a newline inside the type (because there is one between the comment and `int`) which then forces a split after the type annotation too:

```dart
// Comment
int
field;
```

In most places, this is handled by having the surrounding SequenceBuilder grab the comment before visiting the subsequent declaration. But that doesn't work when you have a comment after metadata:

```dart
@meta
// Comment
int field;
```

Now the comment isn't before the field declaration. It's stuck inside it. But we still want to hoist it out.

This PR fixes that for every place I could find: fields, functions, variables, and for-loop variables. The latter is particularly hard because for-in loops have some weird formatting already and it's also just a weird place for splits to occur.

I wish I had a cleaner more systematic way of handling these but despite trying for most of today, I haven't been able to come up with a cleaner approach. This at least gets the formatter producing better output.

Fix #1604.
munificent added a commit that referenced this issue Dec 7, 2024
Handle comments and metadata before variables more gracefully.

Comments and metadata before a declaration are always tricky because the formatter by default attaches comments to the innermost piece following the comment. So in:

```dart
// Comment
int field;
```

We want the comment attached to the Piece for the entire field declaration. But the formatter won't see the comment until it hits the `int` token and will end up attaching it to the piece for the type annotation which is embedded inside the field piece. That in turn means that the formatter will think there is a newline inside the type (because there is one between the comment and `int`) which then forces a split after the type annotation too:

```dart
// Comment
int
field;
```

In most places, this is handled by having the surrounding SequenceBuilder grab the comment before visiting the subsequent declaration. But that doesn't work when you have a comment after metadata:

```dart
@meta
// Comment
int field;
```

Now the comment isn't before the field declaration. It's stuck inside it. But we still want to hoist it out.

This PR fixes that for every place I could find: fields, functions, variables, and for-loop variables. The latter is particularly hard because for-in loops have some weird formatting already and it's also just a weird place for splits to occur.

I wish I had a cleaner more systematic way of handling these but despite trying for most of today, I haven't been able to come up with a cleaner approach. This at least gets the formatter producing better output.

Fix #1604.
@munificent munificent reopened this Dec 12, 2024
@munificent
Copy link
Member

I'm going to re-open this because users keep being surprised by this style, which suggests that maybe we should go back to indenting fields/variables if the type annotation splits.

I'm not sure if this also means we should indent functions if the return type splits. I'll do some investigation.

@mattrberry
Copy link

It's consistent with things like metadata annotations and doc comments which also go on preceding lines but don't cause the variable or function name to be indented.

One way in which this differs is that annotations and comments are easier to distinguish from the front of the line. If I'm reading a line that starts with @ or //, I know what I'm looking at. When it comes to variable names, the new format requires me to scan to the end of the prior line to tell whether I'm looking at a variable name or a statement.

Just a thought as to why this jumped out to me. I agree, though, that neither solution is perfect and that I could probably get used to either :)

@munificent munificent changed the title Tall style: field formatting Indentation on split variable declarations 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

Successfully merging a pull request may close this issue.

3 participants