Page MenuHomePhabricator

Properly cache terminal width and dirty on SIGWINCH
ClosedPublic

Authored by epriestley on Oct 2 2017, 7:01 PM.
Tags
None
Referenced Files
F13144239: D18669.diff
Fri, May 3, 8:03 AM
F13141247: D18669.diff
Fri, May 3, 4:26 AM
Unknown Object (File)
Thu, Apr 25, 2:01 AM
Unknown Object (File)
Fri, Apr 19, 6:51 PM
Unknown Object (File)
Apr 2 2024, 3:29 PM
Unknown Object (File)
Mar 7 2024, 12:59 PM
Unknown Object (File)
Feb 15 2024, 2:19 AM
Unknown Object (File)
Feb 14 2024, 12:35 PM
Subscribers
None

Details

Summary

See PHI110. Currently, the cache here is bad: if tput fails, we'll keep running tput over and over again, since null is not cached.

Instead:

  • Cache null.
  • Dirty the cache when we receivew SIGWINCH, which indicates the window/terminal metadata has changed.
Test Plan
  • Ran scripts/test/progress_bar.php in wide and narrow windows, saw it fit to the window.
  • Resized the window while it was running, saw it (mostly) figure it out (it can leave some artifacts behind, but we can't do much about that).
  • Added some debugging "print" statements to make sure the cache was working, saw it only recalculate once and then after a resize.

Diff Detail

Repository
rPHU libphutil
Branch
winch1
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/console/PhutilConsoleMetrics.php:41XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 18607
Build 25064: Run Core Tests
Build 25063: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/console/format.php
217–218

Maybe keep this comment? I was briefly confused by this too.

This revision is now accepted and ready to land.Oct 2 2017, 7:51 PM
src/console/format.php
217–218

Oh, sure -- I originally thought I could write this in a slightly less dumb way and make what was going on a little more obvious, but PhutilExecPassthru doesn't currently have a mode to pass some kind of normal pipe for stdout/stderr.

This revision was automatically updated to reflect the committed changes.