Skip to content

Conversation

@MahdiBM
Copy link
Collaborator

@MahdiBM MahdiBM commented Dec 14, 2024

The script is configured to respect a .swiftformatignore file which I'll need for Penny.
swift-format might soon have this ability in itself: swiftlang/swift-format#873

@MahdiBM MahdiBM requested a review from gwynne as a code owner December 14, 2024 03:42
@@ -1,4 +1,5 @@
import XCTest

Copy link
Member

Choose a reason for hiding this comment

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

Really? A spurious newline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually can't find out why this wasn't reported before. This is needed based on how swift-format behaves.

Did some tests locally, it would sometimes randomly emit a warning about this, and sometimes not.
I couldn't find out what makes it care about that line, and what makes it not. Tried different configurations as well
...

Copy link
Member

@gwynne gwynne Dec 14, 2024

Choose a reason for hiding this comment

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

I'm strongly against needing the newline, frankly... If swift-format is demanding it, to me that's a bug in swift-format. Imports should be grouped together unless there are a lot of them and they fall naturally into sectioning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are at a point of "it is what it is" with swift-format. I'd suggest you give up as well. The configuration that Apple repos use also has the same behavior IIRC.

I think it requires the new line because the next import is a @testable one.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, obviously it does but to me it's visually jarring and completely pointless to have that gap. Why should @testable imports be separated?

(I know, I know, swift-format isn't gonna change... but there should really be more control.)

Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

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

I must say there are a lot of ways in which I don't like the direction swift-format has taken. For all its issues, swiftformat always impressed me more with its flexibility.

@gwynne gwynne merged commit 1fd5aca into vapor:main Dec 14, 2024
11 checks passed
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