Repository navigation
[Console] Count wide characters as two columns when wrapping and truncating - #67890
Conversation
…cating OutputWrapper counts characters, so SymfonyStyle blocks with CJK text were up to twice as wide as the line length. Table titles, the suggestions of ambiguous commands and FormatterHelper::truncate() measure in columns but cut in characters, which has the same effect. The suggestions of ambiguous commands also padded names with str_pad(), which counts bytes. OutputWrapper measures each distinct character above U+10FF once, and a text that already contains the filler character (U+FDD0) wraps as before instead of losing that character. Text without wide characters wraps with the same pattern as before. Also ignore "truncat" in codespell: FormatterHelperTest expects that truncated word.
67cc445 to
333af0f
Compare
|
Thank you @b84330808. I pushed a few changes. The wrapper now measures each distinct wide character once, so wrapping 1KB of CJK text takes 0.1ms instead of 1.8ms. A text that already contains U+FDD0 now keeps that character. I also adjusted the new tests so that they merge up to 8.x unchanged. |
|
Thanks for the update. I measured 333af0f against its 6.4 base (Console suite green on PHP 8.5 and on 8.1 with symfony/string 6.4; the 6 behaviour tests fail on the base as expected). The wrapper change holds up: 1KB of CJK wraps in 36µs instead of 507µs here, and texts containing U+FDD0 wrap exactly as on 6.4. Text made of distinct characters gets no benefit (10KB of distinct ideographs: 51µs on 6.4, 5.0ms now), which looks acceptable for console messages.
The width only grows with the length, so a binary search returns the same result. It gave identical output on 36,001 random cases (ASCII, CJK, emoji, ZWJ sequences, combining marks, control characters, newlines, U+FDD0, widths from -7 to 100), and the Console suite passes with it: public static function substrToWidth(?string $string, int $width): string
{
$substring = self::substr($string, 0, $width);
if ($width <= 0 || self::width($substring) <= $width) {
return $substring;
}
// the width grows with the length: search the longest beginning that fits
$min = 0;
$max = $width - 1;
while ($min < $max) {
$length = ($min + $max + 1) >> 1;
if (self::width(self::substr($string, 0, $length)) <= $width) {
$min = $length;
} else {
$max = $length - 1;
}
}
return self::substr($string, 0, $min);
} |
|
Thank you @b84330808. |
…a binary search (nicolas-grekas) This PR was merged into the 6.4 branch. Discussion ---------- [Console] Find the longest prefix that fits a width with a binary search | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Follow up of #67890, as suggested by `@phcorp` there: `Helper::substrToWidth()` dropped one character per iteration and measured the whole prefix each time. `FormatterHelper::truncate()` on 10KB of CJK text with a length of 2000 took 1.1s instead of 2.8ms before #67890. The width only grows with the length, so a binary search finds the same prefix. It returns the same strings as the loop on 20k random inputs mixing ASCII, CJK, emoji, combining marks and control characters. Commits ------- b5a9a00 [Console] Find the longest prefix that fits a width with a binary search
…t (nicolas-grekas) This PR was merged into the 6.4 branch. Discussion ---------- [Console] Fix the width of tags and URLs when wrapping text | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Found while reviewing #67890. `OutputWrapper` counts each formatting tag as one column, but tags print nothing. So a line ends one column too early for each tag on it. That's why #67890 changed the first line of `output_13.txt`. Before #67890, the extra column of the `<comment>` tag compensated for the 💕 emoji, which was counted as one column. This PR restores that line. A URL is also counted as one column, whatever its length: ```php (new OutputWrapper())->wrap('See https://symfony.com/doc/current/console.html for more information', 20); // before: "See https://symfony.com/doc/current/console.html for more\ninformation" // after: "See\nhttps://symfony.com/doc/current/console.html\nfor more information" ``` URLs are still never cut. A URL longer than the width now gets a line of its own. The start of its word stays with the URL when it fits, like the `(` of `(https://...)`. <details><summary>Merge-up notes</summary> 7.4 and 8.1: clean, on top of the pending merge-up of #67890. That merge-up needs `addCommand()` instead of `add()` in the new `ApplicationTest` case on 7.4. 8.2 conflicts with #65359. There, each tag and URL is replaced with one placeholder character before wrapping. I'd resolve it this way. Only tags are replaced, all with the same unused character, and they are put back in order after wrapping. URLs aren't replaced anymore. When the text has tags, the row repeats `(?:<marker>*+.)` instead of `.`. When the text has URLs, the cut of a long word checks for a URL at each character. PCRE compiles these repeated groups up to a width of about 1000, so the width is capped at 900 for such texts. Plain text keeps the 65535 cap. The "Width above the PCRE compiled-size limit" case of `OutputWrapperTest` gets the new expectation. </details> Commits ------- 5ce00d0 [Console] Fix the width of tags and URLs when wrapping text
…8 columns (nicolas-grekas) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Console] Fix wrapping escaped tags and text wider than 138 columns | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Follow up of #67897. `OutputFormatter` prints `\<int>` as text, but `OutputWrapper` read a tag there. Since #67897, a tag takes no column, so the line overflows: ```php (new OutputWrapper())->wrap('Expected array\<int> here', 12); // before: "Expected\narray\<int> here" (15 columns) // after: "Expected\narray\<int>\nhere" ``` Looking for a tag at each escaped `<` was also quadratic. With JIT, a message of about 60KB exhausted the JIT stack, and `$io->error()` printed an empty `[ERROR]` block. The first commit backports a fix of #65359 to 6.4: `wrap()` returned an empty string above 138 columns (148 with `allowCutUrls`). The regex got too large, because PCRE copies the tag and URL patterns for each column. They're now subroutine calls, and a text with tags or URLs wraps at 900 columns at most. I put both commits in one PR because they change the same tag pattern. Two expectations of `OutputWrapperTest` change: `\</b\>` and `\<b>` are text now, so they take their width. <details><summary>Merge-up notes</summary> 7.4 and 8.1: clean, on top of the pending merge-ups of #67890 and #67897. 8.2 conflicts with the fold of #65359. I'd keep the 8.2 side and add `(?<!\\)` before the `<` of the tag pattern that the fold uses. The test file then gets the third width case. Taking the 8.1 code instead also passes, with the malformed UTF-8 scrub of 8.2 and `imux` instead of `$modifiers`. With JIT, it's about three times faster on tagged text. </details> Commits ------- 5f05d1a [Console] Fix wrapping escaped tags and text wider than 138 columns
OutputWrappercounts characters, so aSymfonyStyleblock with CJK text can be about twice as wide as the line length, and the terminal wraps it again. Table titles, the suggestions of ambiguous commands andFormatterHelper::truncate()measure in columns but cut in characters, so they overflow the same way.A wide character is followed by a placeholder while wrapping, so the existing regex counts both of its columns. Fixture
output_13.txtchanges because its two emoji now count as two columns each. The first line of this fixture filled the width exactly. But the wrapper also counts the<comment>tag as a column, so the line now wraps one word earlier.