Page MenuHomePhabricator

Introduce PhutilConsoleView for rendering elements to the console
ClosedPublic

Authored by epriestley on Sep 21 2015, 6:30 PM.
Tags
None
Referenced Files
F13993499: D14136.id34169.diff
Tue, Oct 22, 11:16 PM
F13983642: D14136.id34158.diff
Sun, Oct 20, 7:48 AM
F13969843: D14136.id34170.diff
Thu, Oct 17, 4:09 AM
F13955152: D14136.diff
Mon, Oct 14, 12:21 AM
Unknown Object (File)
Fri, Oct 11, 8:00 AM
Unknown Object (File)
Fri, Oct 11, 8:00 AM
Unknown Object (File)
Fri, Oct 11, 8:00 AM
Unknown Object (File)
Fri, Oct 11, 8:00 AM
Subscribers

Details

Summary

Ref T8243. Ref T9455. Two general issues:

  • Some day, we need to deal with users putting console escape sequences in task titles, etc., and doing mischief on the CLI (this is T8243). This is very very low severity.
  • The phutil_console_x() / pht() / sprintf() / echo / PhutilConsole APIs are all really gross and inconsistent.

Try to make a positive step toward resolving these problems:

  • Introduce ConsoleView, which is like AphrontView but for console output (automatic escaping rules, etc).
  • Convert the existing ConsoleTable to be a ConsoleView.
  • Add a new ConsoleList (list of items) and ConsoleBlock (list of other views, approximately).
  • Get all the escaping working properly (API is a bit rough).

This needs more work eventually but lets us render console UIs that look and work like web UIs from an API standpoint.

Test Plan
  • Ran a bunch of commands that use PhutilConsoleTable and all the output looked OK.
  • See next diff for List and Block.

Diff Detail

Repository
rPHU libphutil
Branch
console1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 8024
Build 9078: [Placeholder Plan] Wait for 30 Seconds
Build 9077: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Introduce PhutilConsoleView for rendering elements to the console.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

Oh, let me reconcile this with the existing PhutilTerminalString from D12966. I forgot that got added.

  • Use existing PhutilTerminalString and tsprintf().
  • The tsprintf() / pht() interaction is probably still going to be iffy, but this should get us clean APIs in at least some cases.
chad edited edge metadata.
This revision is now accepted and ready to land.Sep 21 2015, 7:19 PM
epriestley edited edge metadata.
  • Convert less aggressively. Make tsprintf() do formatting.
This revision was automatically updated to reflect the committed changes.
vrana added inline comments.
src/console/view/PhutilConsoleConcatenatedView.php
17

There's no such method.

src/console/view/PhutilConsoleConcatenatedView.php
17

Hello again!

I'm not sure we actually end up needing this class at all -- it doesn't seem to have ended up with any callsites, and drawConsoleString() does approximately the same thing. Did you run into a use case?

(Or are you identifying these issues via static analysis? The one in D14512 feels like something unrelated to this but I'd guess a static analyzer could catch both...)

Hi again!

Yes, this comes from PHPStan. Where I wasn't sure about the fix, I just commented on the change. Feel free to fix it or ignore it.