-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
replace asserts with nicer ValueErrors in gyb #106
Conversation
Thanks! Please clean up the patch so that there are no unrelated whitespace changes. |
54c6880
to
ebd253b
Compare
@gribozavr Done! |
@@ -464,10 +464,14 @@ def tokenGenerator(self, baseTokens): | |||
if (kind == 'gybBlockOpen'): | |||
# Absorb any '}% <optional-comment> \n' | |||
m2 = gybBlockClose.match(self.template, closePos) | |||
assert m2, "Invalid block closure" # FIXME: need proper error handling here. | |||
if m2 is None: |
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.
Is this equivalent? I thought the equivalent would be if not m2:
.
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.
It would be also great to add a test for this.
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.
Yeah that would be better, fixing.
ebd253b
to
089a967
Compare
|
||
if context.localBindings['__children__'] is not self.children: | ||
raise ValueError( | ||
"Expected bindings to be {}, got {}".format( |
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 error message isn't precise enough. It should say, I think, "The code is not allowed to mutate __children__
".
Here's a test:
%{
__children__ = 42
}%
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.
Sure, that makes sense.
@JHoloboski Thank you! Would you mind squashing 5 commits into one and rebasing on top of current master? |
8c1472e
to
7fb230a
Compare
@gribozavr Done :) |
replace asserts with nicer ValueErrors in gyb
Did anyone look at the actual results of this? What does the "nicer feedback" look like? |
define SWIFT_CC(swift) macro for DispatchStubs
define SWIFT_CC(swift) macro for DispatchStubs Signed-off-by: Daniel A. Steffen <[email protected]>
Disable grouping temporarily because there are no associated group for lit-test-helper
XFAIL Plank and R
Rename _MatchingEngine to _RegexParser.
Saw the fixme in this file on the assert, so: