Add live duration to active running command#10757
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds live elapsed duration text for currently executing terminal blocks by deriving an in-progress duration from start_ts and wrapping the rendered duration in an element that schedules periodic repainting.
Concerns
- Silent commands can miss the repaint loop entirely because the duration is hidden until at least one whole second has elapsed, so the first post-preexec render does not mount the repainting element.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| self.start_ts.and_then(|start| { | ||
| let elapsed = now.signed_duration_since(start); | ||
| let whole_secs = elapsed.num_seconds(); | ||
| (whole_secs > 0).then(|| Duration::seconds(whole_secs)) |
There was a problem hiding this comment.
None for the first second means a silent command’s initial post-preexec render will not include LiveDurationElement, so no repaint_after tick is scheduled and the live counter may never appear until unrelated output or interaction triggers another repaint.
There was a problem hiding this comment.
This review might have been kicked off before the refactor. We do handle this case line 22374 in terminal view
Description
The active block now gets a live duration counter that is rounded to the nearest second and repainted every second.
It does this by adding a new
LiveElementwrapper that handles repainting on an interval. On the next repaint, we can end up removing theLiveElementwrapper which ends the loop.Linked Issue
Closes #9801
ready-to-specorready-to-implement.Testing
./script/runScreenshots / Videos
https://www.loom.com/share/a5337675f17f4b99b5c09aab6369c5f3
Note the tooltip clipping issue is fixed by #10423
Not in loom but also tested that shared session viewers get the live counter
Changelog Entries for Stable
CHANGELOG-NEW-FEATURE: Added live duration to active running command