[ty] Abort printing diagnostics when pressing Ctrl+C#22083
Merged
MichaReiser merged 3 commits intomainfrom Dec 23, 2025
Merged
Conversation
MichaReiser
added
cli
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
MichaReiser
commented
Dec 19, 2025
| let sep = fmt_styled(":", stylesheet.separator); | ||
| for diag in diagnostics { | ||
| if self.config.is_canceled() { | ||
| return Ok(()); |
Member
Author
There was a problem hiding this comment.
It feels a bit bad that we return Ok here but changing it is a bit annoying in upstream code because it prevents us from implementing Display for DisplayDiagnostics.
Member
Author
|
Claude claims that ty should exit with 130 after an interrupt. I don't feel like I know enough whether it's worth the complexity or not |
MichaReiser
requested review from
carljm,
dcreager and
sharkdp
as code owners
MichaReiser
force-pushed
the
micha/handle-ctrl-c-print-diagnostics
branch
from
fbe2478 to
9b37298
Compare
Member
Author
|
I feel pretty good about the approach taken in this change so I'll go ahead and merge this PR tomorrow unless someone objects |
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
1 participant
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.
Summary
ty has good handling for handling CTRL+C during type checking. However, pressing CTRL+C is completely ignored when printing diagnostics. ty happily keeps spamming your terminal until it has printed all diagnostics (how rude of you to interrupt it while doing this important job). This can be especially frustrating if there are many diagnostics.
The reason our CTRL+C handling doesn't work when printing diagnostics is that printing diagnostics happens on the main thread, which is also the one that sets the cancellation flag in the Salsa DB when we receive the CTRL+C signal. That means the CTRL+C signal is queued but processed only after printing all diagnostics.
We could consider moving diagnostic printing off the main thread but I decided against this because:
&mut Dbwhen we start applying fixes. That requires fixes to run on the main thread, and we also want to react to cancellation in that case.salsa::Cancelled::catchwhich I'm not a huge fan of.This is why this PR introduces a new
CancellationTokenthat we can use when running expensive operations on the main thread, without relying on Salsa's cancellation mechanism.Test Plan
Tested that pressing Ctrl+C aborts printing diagnostics.