Clear console before running command in exec -w #10983
Conversation
| @@ -10,16 +10,16 @@ between each change to its code. | |||
| > EOF | |||
|
|
|||
| $ dune exec --watch ./foo.exe & | |||
| Success, waiting for filesystem changes... | |||
| changes... | |||
There was a problem hiding this comment.
Why does it remove the entire sentence, minus the last word, here?
There was a problem hiding this comment.
No idea! It only happens in cram tests - not when running dune from a terminal. I've tried various Console functions to see if I could fix the test behaviour while also maintaining the behaviour when you run the command interactively but nothing works. Maybe something to do with how cram tests capture the output of commands? @rgrinberg any ideas?
There was a problem hiding this comment.
No idea, sorry. But shouldn't you restore the terminal after the process finishes?
There was a problem hiding this comment.
It's non-trivial to restore the terminal after the process finishes because dune doesn't wait for the process to finish and continues to run in watch mode, printing to the terminal itself while the new process runs in the background. This means that running programs that change terminal settings (such as programs with tuis) from exec watch mode will have a bad ux, especially because after every change dune will kill the process and launch a new process, possibly leaving the terminal in a bad state. It's definitely something we could improve in the future but not necessary to solve this issue.
rgrinberg
force-pushed
the
fix-10922
branch
2 times, most recently
from
5a89166
to
763a085
Compare
bin/exec.ml
Outdated
| |> Fiber.map | ||
| ~f: | ||
| (Result.map ~f:(fun prog -> | ||
| Console.reset (); |
There was a problem hiding this comment.
Rather than doing this directly, let's be consistent with dune build watch mode.
| Console.reset (); | |
| Scheduler.maybe_clear_screen ~details_hum:[] config; |
You will need to thread config from run_eagar_watch.
There was a problem hiding this comment.
Thanks for this suggestions. This somehow fixed the issues in tests where the success messages was being truncated.
There was a problem hiding this comment.
Oh I see, because maybe_clear_screen has different behaviour when running inside the dune repo than when running in the wild.
|
Are we aware that all the |
Yep, there's a big comment here explaining why (they're flakey). |
This fixes an issue where the build progress stays on the console after the build completes, and the first line of the output of the command being exec'd is printed on teh same line as the build progress. Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
This fixes an issue where the build progress stays on the console after the build completes, and the first line of the output of the command being exec'd is printed on teh same line as the build progress.
A side effect of this change is that some of the "Build Successful" messages in tests get truncated.
Fixes #10922