Stop goofy formatting when newlines are present already. #5582
Conversation
dereuromark
force-pushed
the
master-hotfix-string-wrap
branch
from
8659b77
to
389f745
Compare
|
Is this fix required in 3.x? |
| @@ -355,6 +355,23 @@ public static function wrap($text, $options = array()) { | |||
| * @return string Formatted text. | |||
| */ | |||
| public static function wordWrap($text, $width = 72, $break = "\n", $cut = false) { | |||
| $paragraphs = explode($break, $text); | |||
| foreach ($paragraphs as &$paragraph) { | |||
| $paragraph = String::_wordWrap($paragraph, $width, $break, $cut); | |||
There was a problem hiding this comment.
I would make me more confortable if you do $paragraph[$i] = String::...
There was a problem hiding this comment.
Why? :) As long as you don't reuse the variables inside that method scope, string references should be rather harmless in PHP.
But I can sure refactor that part.
There was a problem hiding this comment.
Makes me imagine that you are passing a string reference into the function. It is just cosmetic.
There was a problem hiding this comment.
Dumb question, couldn't you just remove all the newlines and then wrap?
There was a problem hiding this comment.
No, because the newlines are intentional in those cases. The text case shows a semantically relevant example.
If newlines are not relevant, I agree, you should remove those first prior to calling the method.
In CLI context people here usually use
$text = 'very long line'
. 'another very long line'
. ...;There was a problem hiding this comment.
But the user has requested the text to be wrapped to a specific length. I guess it is debatable whether the result should be a block of fully justified text though.
There was a problem hiding this comment.
Yeah, but it can easily be user-adjusted to be fully justified by removing the newline characters before and STILL allow the custom newlines for other use cases. But as of now it can't do the latter at all. And for the first you would need the manual removal anyway either way.
|
Good job! 👍 |
|
Yes, it would also need to be fixed in 3.x. |
Stop goofy formatting when newlines are present already.
Forward port fix for wordwrap and newlines (#5582).
Currently the result would be
Now, with this hotfix patch, it is (as expected):
Resolves #5580