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] Incorrect indentation with multiple annotations on a parameter #1461

Closed
munificent opened this issue May 3, 2024 · 0 comments · Fixed by #1478
Closed

[tall] Incorrect indentation with multiple annotations on a parameter #1461

munificent opened this issue May 3, 2024 · 0 comments · Fixed by #1478
Labels
easy rare safe tall Issues related to the new "tall" style (#1253) wrong

Comments

@munificent
Copy link
Member

I'm migrating regression tests and the tall style produces:

greet(
  @Rest(valueHelp: 'who', help: 'Name(s) to greet.') List<String> who, {
  @Group.start(title: 'Output')
      @Option(help: 'How many !\'s to append.')
      int enthusiasm:
      0,
  @Flag(abbr: 'l', help: 'Put names on separate lines.') bool lineMode: false,
  @Option(name: 'greeting', help: 'Alternate word to greet with e.g. "Hi".')
      String salutation:
      'Hello',
}) {}

There are several things wrong here:

  • The subsequent @Option annotations shouldn't be indented.
  • The parameter shouldn't be indented.
  • There's no reason to split at the :.
@munificent munificent added rare easy safe wrong tall Issues related to the new "tall" style (#1253) labels May 3, 2024
munificent added a commit that referenced this issue May 9, 2024
The AST node for a parameter with both metadata and a default value
looks like:

    DefaultValueFormalParameter
      FormalParameter

The inner node has the metadata and the outer one has the default value.
Prior to this PR, that meant that we'd create an AssignPiece for the
default value whose left-hand side piece was the function parameter and
its metadata. That meant that when the metadata split, the outer
AssignPiece would be forced to split.

This slightly awkward change makes the piece structure reflect how the
user (and, alas, not Analyzer) thinks of the syntax where the metadata
is part of the entire parameter and the default value is inside the
parameter.

Fix #1461.
munificent added a commit that referenced this issue May 13, 2024
…es. (#1478)

Correct indentation on parameters with both metadata and default values.

The AST node for a parameter with both metadata and a default value
looks like:

    DefaultValueFormalParameter
      FormalParameter

The inner node has the metadata and the outer one has the default value.
Prior to this PR, that meant that we'd create an AssignPiece for the
default value whose left-hand side piece was the function parameter and
its metadata. That meant that when the metadata split, the outer
AssignPiece would be forced to split.

This slightly awkward change makes the piece structure reflect how the
user (and, alas, not Analyzer) thinks of the syntax where the metadata
is part of the entire parameter and the default value is inside the
parameter.

Fix #1461.

Co-authored-by: Nate Bosch <nbosch@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy rare safe tall Issues related to the new "tall" style (#1253) wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant