Open Alx-Lai opened 1 month ago
Hi @Alx-Lai, oh wow, this patch looks great! Thanks a lot for looking into this, I would very much like to have this feature implemented.
I gave the PR a quick test run locally and this is how it looks:
https://github.com/user-attachments/assets/af54a9f2-b589-4666-b3c7-ee1ac6c5df9e
This is nearly perfect. A few comments:
Could you match and remove the three lines that appear when things compile correctly? The lines which say:
Compiling playground v0.0.1 (/playground)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.60s
Running `target/debug/playground`
By removing them in the happy case, we end up with the same output as today, which helps limit the amount of vertical space used. This is important for a live class where it's very disturbing to scroll up and down :)
Could you avoid showing the stderr
block immediately when compilation starts? Ideally, there would be a single output with the combination of stdout
and stderr
, but I understand that the Playground delivers the data in two separate fields.
Similarly, the block with the stderr
output should be hidden/cleared when the code is recompiled.
I looked briefly at the full Playground and discovered that they use a WebSocket now to stream the output line by line! They include markers to show if this is a stdout
or stderr
:
I would be happy to merge this PR as a first step — I don't know how difficult it will be to adapt the mdbook
code to use a WebSocket as well.
Ok, I'll spend time looking into it. Thanks for the feedback!
Updated the commit.
Hi @Alx-Lai, thanks for the update! I haven't forgotten about it, I'm just slow to review the change.
I'll try asking internally if someone else can help review this.
I just tested the updated version, and it works great!
Add a stderr block. Pros:
Applies patches from rust-lang/mdBook#1315 since the original change was not merged by rust-lang.
Issue: #531