-
Notifications
You must be signed in to change notification settings - Fork 441
Update swift syntax #1217
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
Update swift syntax #1217
Conversation
fb801de
to
a790d59
Compare
@@ -26,7 +26,7 @@ open class BasicFormat: SyntaxRewriter { | |||
Trivia(pieces: [.newlines(1), indentation]) | |||
} | |||
|
|||
private var lastRewrittenToken: TokenSyntax? | |||
private var lastRewrittenToken: TokenSyntax? |
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.
Could we in basic format make if next token == nil return no trailing space
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.
There are a few regressions in terms of formatting IMO. It would be nice to fix them in BasicFormat
in the future but this doesn’t have to block this PR.
@@ -48,13 +48,13 @@ open class BasicFormat: SyntaxRewriter { | |||
open override func visit(_ node: TokenSyntax) -> TokenSyntax { | |||
var leadingTrivia = node.leadingTrivia | |||
var trailingTrivia = node.trailingTrivia | |||
if requiresLeadingSpace(node) && leadingTrivia.isEmpty && lastRewrittenToken?.trailingTrivia.isEmpty != false { | |||
if requiresLeadingSpace(node) && leadingTrivia.isEmpty && lastRewrittenToken? .trailingTrivia.isEmpty != 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.
😬 This seems like a regression. It’s fine now but it would be nice to fix this in BasicFormat for the future
@@ -28,7 +28,7 @@ extension AccessorDecl { | |||
/// A convenience initializer that allows: | |||
/// - Initializing syntax collections using result builders | |||
/// - Initializing tokens without default text using strings | |||
public init(leadingTrivia: Trivia? = nil, unexpectedBeforeAttributes: UnexpectedNodes? = nil, attributes: AttributeList? = nil, unexpectedBetweenAttributesAndModifier: UnexpectedNodes? = nil, modifier: DeclModifier? = nil, unexpectedBetweenModifierAndAccessorKind: UnexpectedNodes? = nil, accessorKind: Token, unexpectedBetweenAccessorKindAndParameter: UnexpectedNodes? = nil, parameter: AccessorParameter? = nil, unexpectedBetweenParameterAndAsyncKeyword: UnexpectedNodes? = nil, asyncKeyword: Keyword?, unexpectedBetweenAsyncKeywordAndThrowsKeyword: UnexpectedNodes? = nil, throwsKeyword: Token? = nil, unexpectedBetweenThrowsKeywordAndBody: UnexpectedNodes? = nil, @CodeBlockItemListBuilder bodyBuilder: () -> CodeBlockItemListSyntax? = { | |||
public init(leadingTrivia: Trivia? = nil, unexpectedBeforeAttributes: UnexpectedNodes? = nil, attributes: AttributeList? = nil, unexpectedBetweenAttributesAndModifier: UnexpectedNodes? = nil, modifier: DeclModifier? = nil, unexpectedBetweenModifierAndAccessorKind: UnexpectedNodes? = nil, accessorKind: Token, unexpectedBetweenAccessorKindAndParameter: UnexpectedNodes? = nil, parameter: AccessorParameter? = nil, unexpectedBetweenParameterAndAsyncKeyword: UnexpectedNodes? = nil, asyncKeyword: Keyword? , unexpectedBetweenAsyncKeywordAndThrowsKeyword: UnexpectedNodes? = nil, throwsKeyword: Token? = nil, unexpectedBetweenThrowsKeywordAndBody: UnexpectedNodes? = nil, @CodeBlockItemListBuilder bodyBuilder: () -> CodeBlockItemListSyntax? = { |
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.
Same here, seems like an unfortunate regression.
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.
leadingTrivia += .space | ||
} | ||
if requiresTrailingSpace(node) && trailingTrivia.isEmpty { | ||
trailingTrivia += .space | ||
} | ||
if let keyPath = getKeyPath(Syntax(node)), requiresLeadingNewline(keyPath), !(leadingTrivia.first?.isNewline ?? false) { | ||
if let keyPath = getKeyPath(Syntax(node)), requiresLeadingNewline(keyPath), !(leadingTrivia.first? .isNewline ?? 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.
We missed some cases here.
Will open a new PR
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.
Closing in favor of #1230. |
No description provided.