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

Change line break formatting in brackets #2072

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

milesziemer
Copy link
Contributor

This commit updates how the formatter decides whether or not to put a line break after an opening bracket and before the closing bracket (bracket meaning (), {}, and []). Previously, all bracket types followed the same logic - which included breaking when there are any array or object descendants. This lead to trait bodies being formatted like:

@someTrait(
    [
        {
	    key: "value"
	}
    ]
)
structure SomeStruct {}

With this change, it will be formatted like:

@someTrait([
    {
        key: "value"
    }
])
structure SomeStruct {}

the same also applies if the trait has a single object member:

@someTrait({
    foo: "bar"
    // ... more members to cause break onto separate lines
})
structure SomeStruct {}

Object nodes now also have a single space of padding inside the brackets when on a single line. Previously, it was like:

metadata foo = {bar: "baz"}

now:

metadata foo = { bar: "baz" }

Finally, when object nodes are elements of an array, they now always line break if not empty. Previously:

metadata foo = [
    {bar: "baz"}
]

now (with above padding changes):

metadata foo = [
    {
        bar: "baz"
    }
]

A few test cases were updated for this behavior, and a few more added to test the nuances of this formatting method.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@milesziemer milesziemer requested a review from a team as a code owner December 12, 2023 22:37
@milesziemer milesziemer requested a review from gosar December 12, 2023 22:37
]

metadata g = [
"abc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: we should in the future reformat this to maybe use a text block

@@ -103,15 +104,38 @@ BracketFormatter forceLineBreaksIfNotEmpty() {
return this;
}

// Don't force line breaks and don't indent inside brackets
BracketFormatter inline(boolean inline) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand what this setter does. Is this really "forceInline"? If so, maybe it should set forceLineBreaks to false, and forceLineBreaks when set to true sets forceInline to false. That way you can remove the other conditionals around it, and the last one set wins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea it is - I made an update. forceInline has no effect if forceLineBreaks is set, so I didn't make forceLineBreaks set forceInline to false. But yea forceInline now sets forceLineBreaks to false, and I got rid of a conditional.

This commit updates how the formatter decides whether or not to
put a line break after an opening bracket and before the closing
bracket (bracket meaning `()`, `{}`, and `[]`). Previously, all
bracket types followed the same logic - which included breaking
when there are any array or object descendants. This lead to trait
bodies being formatted like:
```
@someTrait(
    [
        {
	    key: "value"
	}
    ]
)
structure SomeStruct {}
```
With this change, it will be formatted like:
```
@someTrait([
    {
        key: "value"
    }
])
structure SomeStruct {}
```
the same also applies if the trait has a single object member:
```
@someTrait({
    foo: "bar"
    // ... more members to cause break onto separate lines
})
structure SomeStruct {}
```
Object nodes now also have a single space of padding inside the
brackets when on a single line. Previously, it was like:
```
metadata foo = {bar: "baz"}
```
now:
```
metadata foo = { bar: "baz" }
```
Finally, when object nodes are elements of an array, they now
always line break if not empty. Previously:
```
metadata foo = [
    {bar: "baz"}
]
```
now (with above padding changes):
```
metadata foo = [
    {
        bar: "baz"
    }
]
```
A few test cases were updated for this behavior, and a few
more added to test the nuances of this formatting method.
@milesziemer milesziemer merged commit 3608c98 into smithy-lang:main Jan 24, 2024
10 checks passed
hpmellema pushed a commit to hpmellema/smithy that referenced this pull request Jan 25, 2024
This commit updates how the formatter decides whether or not to
put a line break after an opening bracket and before the closing
bracket (bracket meaning `()`, `{}`, and `[]`). Previously, all
bracket types followed the same logic - which included breaking
when there are any array or object descendants. This lead to trait
bodies being formatted like:
```
@someTrait(
    [
        {
	    key: "value"
	}
    ]
)
structure SomeStruct {}
```
With this change, it will be formatted like:
```
@someTrait([
    {
        key: "value"
    }
])
structure SomeStruct {}
```
the same also applies if the trait has a single object member:
```
@someTrait({
    foo: "bar"
    // ... more members to cause break onto separate lines
})
structure SomeStruct {}
```
Object nodes now also have a single space of padding inside the
brackets when on a single line. Previously, it was like:
```
metadata foo = {bar: "baz"}
```
now:
```
metadata foo = { bar: "baz" }
```
Finally, when object nodes are elements of an array, they now
always line break if not empty. Previously:
```
metadata foo = [
    {bar: "baz"}
]
```
now (with above padding changes):
```
metadata foo = [
    {
        bar: "baz"
    }
]
```
A few test cases were updated for this behavior, and a few
more added to test the nuances of this formatting method.
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 this pull request may close these issues.

2 participants