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

feat: upgrade monolog/monolog to v3 #332

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .envrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
use flake
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ log/html.log
composer.lock
/.php-cs-fixer.cache
.phpunit.result.cache
.direnv
6 changes: 3 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
"minimum-stability": "dev",
"prefer-stable": true,
"require": {
"php": ">=7.4",
"php": ">=7.4|>=8.1",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is superfluous.

"ext-curl": "*",
"ext-tidy": "*",
"fossar/htmlawed": "^1.2.8",
"guzzlehttp/psr7": "^2.0",
"j0k3r/graby-site-config": "^1.0.147",
"j0k3r/httplug-ssrf-plugin": "^2.0",
"j0k3r/php-readability": "^1.2.9",
"monolog/monolog": "^1.18.0|^2.3",
"j0k3r/php-readability": "dev-master",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason you are bumping readability?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of psr/log, it's detailed in the PR description

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because readability 1x only supports psr/log 1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I try 2.0.2?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can put ^2.0 it'll be enough for the psr/log v3

"monolog/monolog": "^2.3|^3",
"php-http/client-common": "^2.5",
"php-http/discovery": "^1.14",
"php-http/httplug": "^2.2",
Expand Down
25 changes: 25 additions & 0 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
description = "Dev environment";

outputs = { self, nixpkgs }:
let pkgs = nixpkgs.legacyPackages.x86_64-linux;
in {
defaultPackage.x86_64-linux =
pkgs.mkShell {
buildInputs = with pkgs; [
(php82.withExtensions
({ enabled, all }: with all; enabled ++ [
tidy
])
)
php82Packages.composer
]; };
};
}
16 changes: 11 additions & 5 deletions src/Monolog/Formatter/GrabyFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace Graby\Monolog\Formatter;

use Monolog\Formatter\HtmlFormatter;
use Monolog\Level;
use Monolog\LogRecord;

/**
* Formats incoming records into an HTML table.
Expand All @@ -18,12 +20,16 @@ class GrabyFormatter extends HtmlFormatter
/**
* Formats a log record.
*
* @param array $record A record to format
* @param array|LogRecord $record A record to format
*
* @return string The formatted record
*/
public function format(array $record): string
public function format($record): string
{
if ($record instanceof LogRecord) {
$record = $record->toArray();
}

$output = '<table cellspacing="1" width="100%" class="monolog-output">';

$output .= $this->addRowWithLevel($record['level'], 'Time', $record['datetime']->format($this->dateFormat));
Expand Down Expand Up @@ -61,18 +67,18 @@ protected function convertToString($data): string
/**
* Creates an HTML table row with background cellon title cell.
*
* @param int $level Error level
* @param Level $level Error level
* @param string $th Row header content
* @param string $td Row standard cell content
* @param bool $escapeTd false if td content must not be html escaped
*/
private function addRowWithLevel(int $level, string $th, string $td = ' ', bool $escapeTd = true): string
private function addRowWithLevel(Level $level, string $th, string $td = ' ', bool $escapeTd = true): string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work with Monolog 2, will it?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least there is a specific build to run on lower deps, so it's checked https://github.com/j0k3r/graby/actions/runs/5375860418/jobs/9752256955?pr=332#step:4:115

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm getting this error, I think I have monolog 2 installed.

Graby\Monolog\Formatter\GrabyFormatter::addRowWithLevel(): Argument #2 ($th) must be of type string, int gi

{
$th = htmlspecialchars($th, \ENT_NOQUOTES, 'UTF-8');
if ($escapeTd) {
$td = '<pre>' . htmlspecialchars($td, \ENT_NOQUOTES, 'UTF-8') . '</pre>';
}

return "<tr style=\"padding: 4px;spacing: 0;text-align: left;\">\n<th style=\"background:" . $this->logLevels[$level] . "\" width=\"100px\">$th:</th>\n<td style=\"padding: 4px;spacing: 0;text-align: left;background: #eeeeee\">" . $td . "</td>\n</tr>";
return "<tr style=\"padding: 4px;spacing: 0;text-align: left;\">\n<th style=\"background:" . $this->getLevelColor($level) . "\" width=\"100px\">$th:</th>\n<td style=\"padding: 4px;spacing: 0;text-align: left;background: #eeeeee\">" . $td . "</td>\n</tr>";
}
}
13 changes: 11 additions & 2 deletions src/Monolog/Handler/GrabyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Graby\Monolog\Formatter\GrabyFormatter;
use Monolog\Handler\AbstractProcessingHandler;
use Monolog\Logger;
use Monolog\LogRecord;
use Monolog\Processor\PsrLogMessageProcessor;

/**
Expand Down Expand Up @@ -45,9 +46,17 @@ public function hasRecords($level): bool
return isset($this->recordsByLevel[$level]);
}

protected function write(array $record): void
/**
* @param array|LogRecord $record
* @return void
*/
protected function write($record): void
{
$this->recordsByLevel[$record['level']][] = $record;
$level = $record instanceof LogRecord
? $record->level->value
: $record['level'];

$this->recordsByLevel[$level][] = $record;
$this->records[] = $record;
}
}