-
Notifications
You must be signed in to change notification settings - Fork 225
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
Adding Guards 👮 #33
Adding Guards 👮 #33
Conversation
var format:Variable? | ||
|
||
let components = token.components() | ||
guard components.count <= 2 else { | ||
throw TemplateSyntaxError("'now' tags should can only have one argument: the format string `\(token.contents)`.") |
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.
Please switch can
to may
(RFC 2119)
This PR still includes the changes for #32, can you remove these from this PR please. |
} | ||
|
||
public init(variable:String, loopVariable:String, nodes:[NodeType], emptyNodes:[NodeType]) { | ||
self.variable = Variable(variable) | ||
self.loopVariable = loopVariable | ||
self.nodes = nodes | ||
// TODO: Handle emptyNodes |
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.
Good spot
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 wonder why the compiler didn't warn about unused variable/parameter btw, I'd expect it to suggest you a Fix-It to replace emptyNodes
with _
in such cases.
Not really actually, as they would conflict if separated: there are |
You can isolate the diff between #32 and this PR here if you want to review it more easily |
Ok, this is now entirely rebased without the commits from #32 and ready to merge! |
Thanks for the pull request! |
This PR goes on top of #32 and convert the code to modern Swift by using
guard
to make it waaayy more readable 😉I kept #32 separated from this PR as they are not directly related and you might want to review them separately, but this PR still requires #32 to be merged first — as the added
guard
statements also apply to changes made by #32