From 0febbfbc9f61555c00c7c1084270b2b2c90a6f8c Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 14 Nov 2023 16:49:25 -0500 Subject: [PATCH 1/4] Fix two line wrapping bugs in default report formatter The first is that the ANSI escape codes applied to bold the message when `-s` is used were not being taken into account when wrapping the lines for width, causing some lines to be wrapped unnecessarily. The second is that when lines were wrapped in the middle of a long message, the `|` characters making up the table were being bolded along with the message. --- src/Reports/Full.php | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/Reports/Full.php b/src/Reports/Full.php index f6db5833be..b0666aedd2 100644 --- a/src/Reports/Full.php +++ b/src/Reports/Full.php @@ -115,6 +115,14 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, // The maximum amount of space an error message can use. $maxErrorSpace = ($width - $paddingLength - 1); + if ($showSources === true) { + $beforeMsg = "\033[1m"; + $afterMsg = "\033[0m"; + } else { + $beforeMsg = ''; + $afterMsg = ''; + } + foreach ($report['messages'] as $line => $lineErrors) { foreach ($lineErrors as $column => $colErrors) { foreach ($colErrors as $error) { @@ -128,23 +136,35 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, $lastLine = (count($msgLines) - 1); foreach ($msgLines as $k => $msgLine) { if ($k === 0) { - if ($showSources === true) { - $errorMsg .= "\033[1m"; - } + $errorMsg .= $beforeMsg; } else { - $errorMsg .= PHP_EOL.$paddingLine2; - } - - if ($k === $lastLine && $showSources === true) { - $msgLine .= "\033[0m".' ('.$error['source'].')'; + $errorMsg .= $afterMsg.PHP_EOL.$paddingLine2.$beforeMsg; } - $errorMsg .= wordwrap( + $wrappedLines = wordwrap( $msgLine, $maxErrorSpace, - PHP_EOL.$paddingLine2 + $afterMsg.PHP_EOL.$paddingLine2.$beforeMsg ); - } + $errorMsg .= $wrappedLines; + + if ($k === $lastLine) { + $errorMsg .= $afterMsg; + if ($showSources === true) { + $lastLineLength = strlen($wrappedLines); + $lastNewlinePos = strrpos($wrappedLines, PHP_EOL); + if ($lastNewlinePos !== false) { + $lastLineLength -= ($lastNewlinePos + strlen(PHP_EOL.$paddingLine2.$beforeMsg)); + } + + if (($lastLineLength + strlen($error['source']) + 3) > $maxErrorSpace) { + $errorMsg .= PHP_EOL.$paddingLine2.'('.$error['source'].')'; + } else { + $errorMsg .= ' ('.$error['source'].')'; + } + } + } + }//end foreach // The padding that goes on the front of the line. $padding = ($maxLineNumLength - strlen($line)); From 3577799f9d7300c462205d07563f6f0429a8e2f0 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 10 Dec 2023 20:47:27 +0100 Subject: [PATCH 2/4] Report Full: iterate on line wrapping bug fix This commit takes the fix one step further by adding the padding only after the message has been word-wrapped. Includes correct handling of padding for multi-line error messages. It then takes the last line of the resulting message and detemines in isolation whether the source code suffix can fit on that line or needs to be placed on a new line. --- src/Reports/Full.php | 69 ++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/Reports/Full.php b/src/Reports/Full.php index b0666aedd2..360bfa8c11 100644 --- a/src/Reports/Full.php +++ b/src/Reports/Full.php @@ -115,56 +115,49 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, // The maximum amount of space an error message can use. $maxErrorSpace = ($width - $paddingLength - 1); + $beforeMsg = ''; + $afterMsg = ''; if ($showSources === true) { $beforeMsg = "\033[1m"; $afterMsg = "\033[0m"; - } else { - $beforeMsg = ''; - $afterMsg = ''; } foreach ($report['messages'] as $line => $lineErrors) { foreach ($lineErrors as $column => $colErrors) { foreach ($colErrors as $error) { - $message = $error['message']; - $msgLines = [$message]; - if (strpos($message, "\n") !== false) { - $msgLines = explode("\n", $message); - } + $errorMsg = wordwrap( + $error['message'], + $maxErrorSpace + ); - $errorMsg = ''; - $lastLine = (count($msgLines) - 1); - foreach ($msgLines as $k => $msgLine) { - if ($k === 0) { - $errorMsg .= $beforeMsg; - } else { - $errorMsg .= $afterMsg.PHP_EOL.$paddingLine2.$beforeMsg; + // Add the padding _after_ the wordwrap as the message itself may contain line breaks + // and those lines will also need to receive padding. + $errorMsg = str_replace("\n", $afterMsg.PHP_EOL.$paddingLine2.$beforeMsg, $errorMsg); + $errorMsg = $beforeMsg.$errorMsg.$afterMsg; + + if ($showSources === true) { + $lastMsg = $errorMsg; + $startPosLastLine = strrpos($errorMsg, $paddingLine2.$beforeMsg); + if ($startPosLastLine !== false) { + // Message is multiline. + $lastMsg = substr($errorMsg, ($startPosLastLine + strlen($paddingLine2.$beforeMsg))); } - $wrappedLines = wordwrap( - $msgLine, - $maxErrorSpace, - $afterMsg.PHP_EOL.$paddingLine2.$beforeMsg - ); - $errorMsg .= $wrappedLines; - - if ($k === $lastLine) { - $errorMsg .= $afterMsg; - if ($showSources === true) { - $lastLineLength = strlen($wrappedLines); - $lastNewlinePos = strrpos($wrappedLines, PHP_EOL); - if ($lastNewlinePos !== false) { - $lastLineLength -= ($lastNewlinePos + strlen(PHP_EOL.$paddingLine2.$beforeMsg)); - } - - if (($lastLineLength + strlen($error['source']) + 3) > $maxErrorSpace) { - $errorMsg .= PHP_EOL.$paddingLine2.'('.$error['source'].')'; - } else { - $errorMsg .= ' ('.$error['source'].')'; - } - } + // When show sources is used, the message itself will be bolded, so we need to correct the length. + $sourceSuffix = '('.$error['source'].')'; + + $lastMsgPlusSourceLength = strlen($lastMsg); + // Add space + source suffix length. + $lastMsgPlusSourceLength += (1 + strlen($sourceSuffix)); + // Correct for the color codes. + $lastMsgPlusSourceLength -= 8; + + if ($lastMsgPlusSourceLength > $maxErrorSpace) { + $errorMsg .= PHP_EOL.$paddingLine2.$sourceSuffix; + } else { + $errorMsg .= ' '.$sourceSuffix; } - }//end foreach + }//end if // The padding that goes on the front of the line. $padding = ($maxLineNumLength - strlen($line)); From b9dd7db37a5f94469eec817bee8baa4ec3f6959d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 11 Dec 2023 01:14:26 +0100 Subject: [PATCH 3/4] Report Full: iterate on line wrapping bug fix [2] Prevent inconsistency in handling of single-line vs multi-line length calculation. Co-authored-by: Brad Jorsch --- src/Reports/Full.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Reports/Full.php b/src/Reports/Full.php index 360bfa8c11..3f99e976ea 100644 --- a/src/Reports/Full.php +++ b/src/Reports/Full.php @@ -137,10 +137,10 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, if ($showSources === true) { $lastMsg = $errorMsg; - $startPosLastLine = strrpos($errorMsg, $paddingLine2.$beforeMsg); + $startPosLastLine = strrpos($errorMsg, PHP_EOL.$paddingLine2.$beforeMsg); if ($startPosLastLine !== false) { - // Message is multiline. - $lastMsg = substr($errorMsg, ($startPosLastLine + strlen($paddingLine2.$beforeMsg))); + // Message is multiline. Grab the text of last line of the message, including the color codes. + $lastMsg = substr($errorMsg, ($startPosLastLine + strlen(PHP_EOL.$paddingLine2))); } // When show sources is used, the message itself will be bolded, so we need to correct the length. From 0ba5736e423c8ba3dafe3ed382aa58b91cc5c315 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 11 Dec 2023 01:25:03 +0100 Subject: [PATCH 4/4] Report Full: iterate on line wrapping bug fix [3] Don't hard code the color code length to prevent this breaking on potential future changes. Co-authored-by: Brad Jorsch --- src/Reports/Full.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Reports/Full.php b/src/Reports/Full.php index 3f99e976ea..ee2b5af203 100644 --- a/src/Reports/Full.php +++ b/src/Reports/Full.php @@ -122,6 +122,8 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, $afterMsg = "\033[0m"; } + $beforeAfterLength = strlen($beforeMsg.$afterMsg); + foreach ($report['messages'] as $line => $lineErrors) { foreach ($lineErrors as $column => $colErrors) { foreach ($colErrors as $error) { @@ -150,7 +152,7 @@ public function generateFileReport($report, File $phpcsFile, $showSources=false, // Add space + source suffix length. $lastMsgPlusSourceLength += (1 + strlen($sourceSuffix)); // Correct for the color codes. - $lastMsgPlusSourceLength -= 8; + $lastMsgPlusSourceLength -= $beforeAfterLength; if ($lastMsgPlusSourceLength > $maxErrorSpace) { $errorMsg .= PHP_EOL.$paddingLine2.$sourceSuffix;