-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add a swift-format configuration file #9005
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci test |
"indentation": { | ||
"spaces": 4 | ||
}, | ||
"lineBreakBeforeEachArgument": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule feels a little too opinionated to me, not sure if anyone else has an opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this option helps make the diff ever-so-slightly cleaner to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would result in a pretty huge diff. While I agree it can enhance readability in some cases, I'm not sure we should make the jump right away. I'd omit this for now and maybe we should have a wider discussion first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to get to the following
func foo(a: Int, b: String) {
}
to
func foo(
a: Int,
b: String,
) {
}
{ | ||
"version": 1, | ||
"lineLength": 10000, | ||
"indentation": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I haven't read the documentation, but based on the setting name, we should consider "multiElementCollectionTrailingCommas" : true,
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That corresponds to:
[
"a",
"b"
]
vs
[
"a",
"b",
]
Support for the latter was added to Swift some time ago, the argument being that it helps reduce the SCM diff as new lines are added to the bottom.
I'd be fine with enabling that rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want the latter :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also want
[1, 3]
to format to
[
1,
3,
]
}, | ||
"lineBreakBeforeEachArgument": true, | ||
"rules": { | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I haven't read the documentation ,but based on the setting name, we should consider the following, eventually:
"rules" : {
"AllPublicDeclarationsHaveDocumentation" : false,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to strive towards, but we should not add that immediately as I think for SwiftPM in particular there's a lot which is public but which maybe shouldn't be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% agree :). Something to consider for the future.
{ | ||
"version": 1, | ||
"lineLength": 10000, | ||
"indentation": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That corresponds to:
[
"a",
"b"
]
vs
[
"a",
"b",
]
Support for the latter was added to Swift some time ago, the argument being that it helps reduce the SCM diff as new lines are added to the bottom.
I'd be fine with enabling that rule.
@@ -0,0 +1,11 @@ | |||
{ | |||
"version": 1, | |||
"lineLength": 10000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with this; we should not enforce a line length.
"indentation": { | ||
"spaces": 4 | ||
}, | ||
"lineBreakBeforeEachArgument": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would result in a pretty huge diff. While I agree it can enhance readability in some cases, I'm not sure we should make the jump right away. I'd omit this for now and maybe we should have a wider discussion first.
}, | ||
"lineBreakBeforeEachArgument": true, | ||
"rules": { | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to strive towards, but we should not add that immediately as I think for SwiftPM in particular there's a lot which is public but which maybe shouldn't be.
@bkhouri Can you open a similar PR for swift-build? |
@jakepetroules there is already a Maybe we should merge this change once we have GitHub actions, where, in one PR, we enforce formatting rules and format the code base. |
No description provided.