-
Notifications
You must be signed in to change notification settings - Fork 18
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: cursor and newline missing when running without --watch #49
base: main
Are you sure you want to change the base?
fix: cursor and newline missing when running without --watch #49
Conversation
return sub.read() | ||
..then((_) => stdout | ||
..write(VT100.showCursor) | ||
..write('\n')); |
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 the extra \n?
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.
Otherwise this problem occurs.
@@ -231,7 +231,12 @@ Future<int> spec({ | |||
final sub = ref.listen($exitCode.future, (previous, current) {}); | |||
|
|||
// Outside of watch mode, quit as soon we know what the exit code should be | |||
return sub.read(); | |||
return () { |
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 function block is unnecessary. You don't have to put everything on the return
keyword
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 rewrote it a bit.
Thanks! |
Do you mind writing a test for this? As an example, you can look at You might have to change |
Fixes #46
Changes proposed
Apparently the
finally
block containingVT100.showCursor
andVT100.newline
was not executed because the function was beingreturn
ed from within thetry
block.Now it's fixed. I didn't write tests for it because I'm not familiar with this project's codebase at all.
Check List (Check all the applicable boxes)