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
Unknown Object (File)
Sun, Mar 17, 5:01 PM
Unknown Object (File)
Sun, Mar 10, 2:52 PM
Unknown Object (File)
Sun, Mar 10, 2:47 PM
Unknown Object (File)
Sun, Mar 10, 2:23 PM
Unknown Object (File)
Sun, Mar 10, 2:19 PM
Unknown Object (File)
Sun, Mar 10, 12:56 PM
Unknown Object (File)
Sun, Mar 10, 12:35 PM
Unknown Object (File)
Thu, Mar 7, 11: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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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
16

There's no such method.

src/console/view/PhutilConsoleConcatenatedView.php
16

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.