bobthecow/psysh

Allow Custom Color Configuration

Closed this issue · 11 comments

Symphony OutputFormatter allows you to override the colors for errors and such.

https://github.com/symfony/console/blob/d487d68fd7e85e2d3fc5cc69e41d66edd15cec1e/Formatter/OutputFormatter.php#L62-L76

Because it is used statically in formatException there is a missed opportunity to override these default colors.

return \sprintf('<%s>%s</%s>', $severity, OutputFormatter::escape($message), $severity);

Why does this matter? The default on my screen, black background and green text (yeah, I'm old), the error is virtually unreadable with "white text on red background." In my ANSI setup, "white" is light gray and "bold white" is white white.

It'd be great to make this user-configurable.

Screen Shot 2020-03-02 at 11 44 15 PM

Bonus Points: it'd be nice to change the default colors too.

private function getDefaultConsoleColor()
{
$color = new ConsoleColor();
$color->addTheme(Highlighter::LINE_NUMBER, ['blue']);
$color->addTheme(Highlighter::TOKEN_KEYWORD, ['yellow']);
$color->addTheme(Highlighter::TOKEN_STRING, ['green']);
$color->addTheme(Highlighter::TOKEN_COMMENT, ['dark_gray']);
return $color;
}

I'm with you. I prefer black text on red background. That's what PsySH used for the longest time. But that also doesn't work for a bunch of people, which is why it changed.

I just pushed a commit to the master branch that adds formatterStyles to the config options. If you install @dev via Composer, you will get that version and be able to play around with it.

Adding something like this to ~/.config/psysh/config.php should do it:

return [
    'formatterStyles' => [
        // name => [fg, bg, [options]]
        'error' => ['black', 'red', ['bold']],
    ],
];

I'd like to test this but the instructions for checking it out of Git and building it leaves me with a script in bin/psysh that is not all-in-one-self-contained.

Do you mind posting instructions for how to build a portable version? Or throw a Makefile in there... :-)

It’s in the current release now, so you can upgrade and just use that :)

Also, there’s a Makefile! 😁

HOLY CRAP there is. I'm blind. Sorry!

Do you need to run gmake dist or can you target just the most-recent portable version?

Got the latest, it's working! One error I found, not handling exceptions.

I put in "normal" in the options list, and this failed (invalid value). However psysh crashed because it was attempting to re-read config.php and it hit a function I defined there and threw a "Cannot redeclare myfunc()" and crashed. This was odd. You might want to add some validation for formatterStyles to make sure the user doesn't totally break psysh unwittingly and without knowing why.

Likely a simple try { } catch around new OutputFormatterStyle will catch the errors thrown by Symphony.

I wish OutputFormatterStyle would make the definitions of the colors accessible, so when throwing an exception you could iterate through the list of valid colors and save the user some hassle looking up what valid values are. (Yes, I see normal is not a valid value)

Thanks!

The error message does list the valid colors (at least with the version of Symfony I'm currently testing against):

Invalid background color specified: "redd". Expected one of (black, red, green, yellow, blue, magenta, cyan, white, default).

I don't know about swallowing errors and continuing. I mean, the current experience far from ideal, but it's pretty clear what went wrong:

Screen Shot 2020-04-08 at 2 24 17 PM

Part of the difficulty is that during the "parse config" stage, we don't even have a Shell instance running, so we can't really log errors and continue. I'll think for a bit about how to provide a better experience. I'd also appreciate your thoughts :)

I'm also planning to update documentation and add examples so nobody has to figure it out by trial and error :)

Try adding a function to your config.php and re-run with the bad option (not a bad color) --

function hiBob() {
  // do nothing
  return true;
}

I got this in my php-errors, not the beautiful fatal error:

[08-Apr-2020 20:08:34 UTC] PHP Fatal error:  Cannot redeclare tdgo() (previously declared in /usr/home/beckman/.config/psysh/config.php:12) in /usr/home/beckman/.config/psysh/config.php on line 12

I suspect that the error may be causing config.php to be re-read, attempting to redeclare a function, thus hiding the fatal error that would have indicated where I messed up.

Ahh, I see what's happening.

As of the latest release, we added more robust command line option parsing, namely this:

psysh/src/functions.php

Lines 335 to 340 in 963ff8b

try {
$config = Configuration::fromInput($input);
} catch (\InvalidArgumentException $e) {
$config = new Configuration();
$usageException = $e;
}

What it's supposed to be doing is creating a Configuration object with your command line options, catching any errors those options throw, and falling back to a default Configuration object (which surely can't fail... right?).

But in your case, the failure is in the config file, not the command line options, so when we swallow the original error and try again, the second one fails too, but this time with a "you already defined this function" error, not the error which caused the config failure to begin with.

We could solve it by falling back to not creating a Configuration object when creating the Shell at all, but funny story:

psysh/src/Shell.php

Lines 82 to 84 in 963ff8b

public function __construct(Configuration $config = null)
{
$this->config = $config ?: new Configuration();

... if we did that, we'd just trigger the error again a split second later when we tried to initialize the Shell :-/

At the very least, you should be defensively defining that function in your config file by wrapping it in if (!\function_exists('...')) .... We absolutely will require that file more than once. For example, if you launch a shell multiple times in a single web request to look at state in different places, we'll need to initialize configuration for each one, which would mean re-requiring that file.

As for what to do about the configuration error explosion, I'll think about that :)

Alright, got it. The next release won't have cascading config failures anymore!

I also added a note to the Configuration page in the wiki about defensively defining things in your config:

https://github.com/bobthecow/psysh/wiki/Configuration#define-things-defensively