-
Notifications
You must be signed in to change notification settings - Fork 441
Improve diagnostics for invalid version tuples #1809
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
Improve diagnostics for invalid version tuples #1809
Conversation
87f5797
to
317ead7
Compare
317ead7
to
6cfad27
Compare
6cfad27
to
74af3c1
Compare
74af3c1
to
ab2a4ad
Compare
f0cec9b
to
6b8e867
Compare
Accidentally clicked approve. I’ve had two more minor requests
6b8e867
to
178eccd
Compare
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.
One minor comment, otherwise LGTM.
var unexpectedTokens: [RawSyntax] = [] | ||
var keepGoing: RawTokenSyntax? = nil | ||
var loopProgress = LoopProgressCondition() | ||
repeat { | ||
if let keepGoing { | ||
unexpectedTokens.append(RawSyntax(keepGoing)) | ||
} | ||
if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) { | ||
unexpectedTokens.append(RawSyntax(unexpectedVersion)) | ||
} | ||
keepGoing = self.consume(if: .period) | ||
} while keepGoing != nil && loopProgress.evaluate(currentToken) |
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.
You could just make unexpectedTokens
a [RawTokenSyntax]
, which simplifies the code a little
var unexpectedTokens: [RawSyntax] = [] | |
var keepGoing: RawTokenSyntax? = nil | |
var loopProgress = LoopProgressCondition() | |
repeat { | |
if let keepGoing { | |
unexpectedTokens.append(RawSyntax(keepGoing)) | |
} | |
if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) { | |
unexpectedTokens.append(RawSyntax(unexpectedVersion)) | |
} | |
keepGoing = self.consume(if: .period) | |
} while keepGoing != nil && loopProgress.evaluate(currentToken) | |
var unexpectedTokens: [RawTokenSyntax] = [] | |
var keepGoing: RawTokenSyntax? = nil | |
var loopProgress = LoopProgressCondition() | |
repeat { | |
if let keepGoing { | |
unexpectedTokens.append(keepGoing) | |
} | |
if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) { | |
unexpectedTokens.append(unexpectedVersion) | |
} | |
keepGoing = self.consume(if: .period) | |
} while keepGoing != nil && loopProgress.evaluate(currentToken) |
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 have committed your suggestion, thank you!
178eccd
to
8ae11b1
Compare
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.
Great, thanks. Let’s see what CI says.
@swift-ci Please test |
Hello @ahoppen 👋 It seems like the CI is completed. When can it be merged? |
Thanks for the ping. I forgot about this PR. I’m sorry. |
Resolve #1613
During the process of rebasing the changes from the main branch, an issue arose, leading to the closure and reopening of the previous PR(#1717).
I sincerely apologize for the inconvenience caused.
Based on the feedback provided in the comment here(#1717 (comment)), I have made the necessary revisions and added several relevant test cases.