-
Notifications
You must be signed in to change notification settings - Fork 8
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
fix @manipulate
for upcoming base changes
#46
Conversation
src/manipulate.jl
Outdated
@@ -79,6 +79,10 @@ macro manipulate(args...) | |||
block = expr.args[2] | |||
if expr.args[1].head == :block | |||
bindings = expr.args[1].args | |||
# remove trailing LineNumberNodes from loop body as to not just return `nothing` |
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'm not 100% sure I understand what will change in newer julia versions, but the loop body is block
, defined a few lines above. expr
is a for
loop, so the body should be its second argument.
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.
Well, that's embarassing... ^^ Yes, you are absolutely right!
src/manipulate.jl
Outdated
@@ -79,6 +79,10 @@ macro manipulate(args...) | |||
block = expr.args[2] | |||
if expr.args[1].head == :block | |||
bindings = expr.args[1].args | |||
# remove trailing LineNumberNodes from loop body as to not just return `nothing` | |||
if bindings[end] isa LineNumberNode | |||
resize!(bindings, length(bindings) - 1) |
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.
Why not just pop!
instead of resize!(v, length(v) - 1)
? I think it'd be a bit more readable.
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, seems reasonable.
Thanks for the PR! For me to understand, the loop body already has some May be worth to also add a link to the PR to julia in the comments in the source code. |
Yes, blocks always return the return value of the last expression and if that's a
done. |
ref JuliaLang/julia#41857