Repository navigation
Conversation
println!() panics when a write fails. reset_sigpipe() handles this on Unix by dying on SIGPIPE, but on platforms without SIGPIPE (e.g. Windows) piping difft's output to a process that exits early crashed with "failed printing to stdout". Thread an io::Result through the display functions so write errors propagate to main, and exit silently on BrokenPipe, mirroring the SIGPIPE termination on Unix. Other write errors still panic, as they did before. This also makes display slightly faster, as we hold a single locked stdout for the duration of printing instead of locking it on every println!. Developed with AI assistance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1068
Summary
reset_sigpipeis a no-op on Windows (#[cfg(unix)]), so when theprocess reading difftastic's stdout exits early —
difft a b | head -2—every
println!path panics:On Unix the same invocation dies quietly to SIGPIPE (exit 141). This PR
makes the non-Unix path match that behavior.
Approach
print!/println!panic inside std on write errors, so there is nointerceptable choke point. Instead this threads a single
&mut impl io::Write(aStdoutLockowned bymain) through the~47 output sites across the display and parse modules, all now returning
io::Result<()>:main()splits intorun(mode, out) -> io::Result<()>; thewrapper maps
ErrorKind::BrokenPipetoprocess::exit(EXIT_SUCCESS)and keeps the identical panic message for other write errors
(parity with std's
failed printing to stdoutmessage).StdoutLockis used (notBufWriter) soprocess::exitpaths can never lose buffered output. As a side benefit there is now
one lock acquisition per result instead of per line.
.expect(\"Receiver should be connected\"); once the receiver candrop early (on a
?bail) that would panic. It now ignores the senderror with an explanatory comment — only reachable when we are already
erroring out.
stderr output (
eprintln!) is intentionally unchanged — panic on aclosed stderr is the status quo on all platforms.
Verification
side-by-side, inline,
--display=json,--dump-syntax,--dump-ts,--list-languages, dir diffs, git unmerged mode —all quiet exit 0 where they previously panicked (verified by
neutralizing the SIGPIPE reset so the Unix build takes the Windows
code path exactly).
x86_64-pc-windows-gnu, run on Windows 11):difft a.py b.py | Select-Object -First 0and-First 2exit 0;normal output unchanged.
inline, json, dump modes, list-languages, exit codes).
cargo fmt --checkclean; clippy shows only pre-existing warnings;cargo test: 123 unit + 24 CLI tests pass;cargo check --target x86_64-pc-windows-gnuclean.Disclosure
Implemented with AI assistance (Devin); the change was reviewed by the
account owner, and verified as described above. See issue #1068.