Skip to content

Update SwiftSyntax 0.50200.0#171

Closed
noppefoxwolf wants to merge 7 commits intoFlineDev:stablefrom
noppefoxwolf:feature/UpdateSwiftSyntax
Closed

Update SwiftSyntax 0.50200.0#171
noppefoxwolf wants to merge 7 commits intoFlineDev:stablefrom
noppefoxwolf:feature/UpdateSwiftSyntax

Conversation

@noppefoxwolf
Copy link
Copy Markdown
Contributor

No description provided.

@alphatroya
Copy link
Copy Markdown

I've run this branch version and can confirm that it working fine. Transform command is working well.

Copy link
Copy Markdown
Member

@Jeehut Jeehut left a comment

Choose a reason for hiding this comment

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

HI @noppefoxwolf, thank you very much for this PR!
I've reviewed your code and found some issues or had some questions. Please handle them, then I'll test this myself and would be happy to merge! 👍


guard
let memberAccessExpression = functionCallExpression.child(at: 0) as? MemberAccessExprSyntax,
let memberAccessExpression = functionCallExpressionIterator.next()?.as(MemberAccessExprSyntax.self), // 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What does the // 0 mean here? Please be more spcific like // index 0. And why is .next() here the same as child(at: 0) before?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because child(at:) was removed from FunctionCallExprSyntax.
But I think too that is not readable easily.
I'll fix using another way.

else {
return super.visit(functionCallExpression)
}
_ = functionCallExpressionIterator.next() // 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my comment above. ⬆️

}
var functionCallArgumentListIterator = functionCallArgumentList.children.makeIterator()


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please never use two newlines, one is sufficient.

}

var translationsFunctionCallArgumentIterator = translationsFunctionCallArgument.children.makeIterator()
_ = translationsFunctionCallArgumentIterator.next() // 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my comment above. ⬆️


var translationsFunctionCallArgumentIterator = translationsFunctionCallArgument.children.makeIterator()
_ = translationsFunctionCallArgumentIterator.next() // 0
_ = translationsFunctionCallArgumentIterator.next() // 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my comment above. ⬆️


if let translationsDictionaryElementList = translationsDictionaryExpression.child(at: 1) as? DictionaryElementListSyntax {
var translationsDictionaryExpressionIterator = translationsDictionaryExpression.children.makeIterator()
_ = translationsDictionaryExpressionIterator.next() // 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See my comment above. ⬆️

@Jeehut
Copy link
Copy Markdown
Member

Jeehut commented Apr 10, 2020

Closing this in favor of #172 where I will fix the leftover comments and merge. Thank you very much @noppefoxwolf for you work here! 👍

@Jeehut Jeehut closed this Apr 10, 2020
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.

3 participants