-
Notifications
You must be signed in to change notification settings - Fork 57
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
with_terminal to capture stdout and stdin #31
Conversation
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.
Thanks for writing it into a PR! We will probably use it in our class too
I have some comments:
src/CodeControl.jl
Outdated
```` | ||
|
||
""" | ||
macro cond(run,expr) |
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 create a macro that does exactly the same as if
? This makes code harder to understand
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 is not exactly the same if I want to use this with a begin/end block.
I can have
@cond test @with_stdout begin
hack hack hack
end
but I must have
if test @with_stdout begin
hack hack hack
end end
with two ends at the end.
But I see some point in your remark (see above).
It comes down to finding the right usage patterns. I envision to explain to new uses the working of pluto before anything else and might explain things both ways. In this sense it appears to be more important to find a proper variable name instead of test
than insisting on @cond.
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 think this macro has 2 (small) advantages over if:
- it replaces both the
if
and theend
, resulting in slightly less typing. - it promotes code execution dependent on a checkbox (or any other bool) to an "official" Pluto usage pattern.
However, I agree that the benefit of it is not huge and therefore it is more a matter of taste. Alternatively, the if-pattern could be documented as officially recommended.
Keeping @cond though so far.
Hi updated the PR. Keeping @cond so far... Added the @capture code I had in the PR to Suppressor (for the time being). Anyway had to modify it as it did work from the REPL and not from Pluto. So now we catch both stdout and stderr. See https://gist.github.com/j-fu/5afae23d4c9676b20b4f0551a97dccd1 . |
For 9cbf04e: not sure about ignoring @async ...
this seems to fill up the stream buffer and getting stuck waiting for taking out some stuff.
|
ConsoleLogger defaults to not printing context info with @info messages Also use now read(stream, String).
I have made some changes:
|
... yeah the function stuff is better, I don't like macros that much anyway. Still wasn't familiar enough with the do-block syntax to get this idea. |
Hi, here is my PR.
Here is an example gist.