Update rust-tui so "cargo run" will not output in terminal#288
Update rust-tui so "cargo run" will not output in terminal#288Chris Draper (Chris-Draper) wants to merge 2 commits into
Conversation
|
The multi-line comment in main.rs might be overkill, but I figured it was better to include it in my initial PR to explain the changes to stderr |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent Ditto SDK C-internal writes to stderr from garbling the Rust TUI during local runs by redirecting stderr to the app’s log file when appropriate.
Changes:
- Redirect
stderrto the configured log file on Unix whenstderris attached to a terminal. - Update
rust-tuiREADME to remove the2>/dev/nullworkaround and instructcargo run. - Add
libcdependency to supportisatty/dup2.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| rust-tui/src/bin/main.rs | Adds Unix-only stderr redirection to the log file before initializing tracing. |
| rust-tui/README.md | Removes the 2>/dev/null workaround and simplifies run instructions. |
| rust-tui/Cargo.toml | Adds libc dependency used by the new Unix-only redirection code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if libc::isatty(libc::STDERR_FILENO) == 1 { | ||
| libc::dup2(logfile.as_raw_fd(), libc::STDERR_FILENO); |
There was a problem hiding this comment.
This seems pretty reasonable. I feel like it can't hurt to have additional error handling in this case. Will leave this comment up for anyone from the Ditto engineering team to take a look.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Hey! I am interviewing for a Product Manager - Networking job at Ditto. I was using your SDK to try out your product.
I wanted to submit a PR for a small quality of life improvement in the rust-tui app
Let me know what you think.