Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Add verbosity level checking to console alerts #44614

Merged
merged 1 commit into from
Oct 17, 2022
Merged

[9.x] Add verbosity level checking to console alerts #44614

merged 1 commit into from
Oct 17, 2022

Conversation

Angel5a
Copy link
Contributor

@Angel5a Angel5a commented Oct 16, 2022

First of all, let me point out some inconsistencies.

Many logging systems and PSR-3 introduces severity level (Syslog, Monolog PSR-3). In most cases they are: emergency, alert, critical, error, warning, notice, info, debug. And all of them are the same in terms, that alert is more important than error.

In Laravel console commands we can control the amount of generated output information by specifying -vvv, -v and -q options. It is provided by the underlying Symfony's Verbosity Levels.
Also Laravel provides several helper functions to colorize the console output: error, warn, info, comment, question. They are utilizing Symfony's Color Styles.
Next we have few useful blocks: alert, table. They can be used to decorate the output.

As you can see several names partially matches severity names: alert, error, warn, info. And the main problem is in alert which is intended to "write a string in an alert box". It gives us a highly visible box, but it lacks the ability to be used as extra important text.

Lets assume this snippet:

class TestSeverity extends Command
{
    protected $signature = 'test:severity';

    public function handle()
    {
        $this->line('Debug message', null, 'vvv');
        $this->line('Detailed report', 'vv');
        $this->info('More info', null, 'v');
        $this->info('Info');
        $this->warn('Warning');
        $this->error('Error');
        $this->error('Error with quiet', 'quiet');
        $this->alert('Alert! Something really dangerous happened!');
        return Command::SUCCESS;
    }
}

Would we run this command with -q (we want to see only important messages) we will see only error:

Error with quiet

And won't see alert at all. Our alert is not an alert, and we can't change this behavior.

The most correct solution would be to rename method alert to box, panel, header or something else. But it would be a breaking change and should be discussed at first place.

Now, about this PR.

Now users can specify verbosity level (importance) for alert boxes (decorated output blocks).

        $this->alert('Alert box only for debug mode', 'vvv');
        $this->alert('Normal alert box');
        $this->alert('Very important alert box', 'quiet');

Boxes are printed based on the current level of output detail. It won't generate dangling blank lines unless the box is printed.

It's not a BC. Currently alert doesn't accept second argument. The parameter has a default value that retains the old behavior. Extra parameter passed to older frameworks will be ignored.

There is no tests. It's still a questionable should we rename this method or not. And, as far as none of this methods were covered with tests, and current fix is trivial, I decide to not write a test that would be removed in the future. But if it is really required, I can provide one. Just let me know.

@taylorotwell taylorotwell merged commit 2e5cbd5 into laravel:9.x Oct 17, 2022
@GrahamCampbell GrahamCampbell changed the title Add verbosity level checking to console alerts. [9.x] Add verbosity level checking to console alerts Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants