-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Formatting code base | ||
| 19ab5e34677a7fea521b682ee35c974f7aaac0fb |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| { | ||
| "version": 1, | ||
| "lineLength": 10000, | ||
bkhouri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "indentation": { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want the latter :)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also want
to format to |
||
| "spaces": 4 | ||
| }, | ||
| "lineBreakBeforeEachArgument": true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to get to the following to |
||
| "maximumBlankLines" : 1, | ||
| "multiElementCollectionTrailingCommas" : true, | ||
| "rules": { | ||
| }, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 100% agree :). Something to consider for the future. |
||
| "tabWidth": 4 | ||
| } | ||
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.
chore (blocking): We need to exclude the
Fixturesdirectory from the formatting check since it contains code that does not compile.