Skip to content

[String] Fix ellipsis of truncate when not using cut option #37054

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

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

duboiss
Copy link
Contributor

@duboiss duboiss commented Jun 1, 2020

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Since 5.1, we can use a cut option on truncate.
But with this option, we don't have the expected behavior when the entire chain is returned.

Currently:
u('Lorem Ipsum')->truncate(8, '…', false); // 'Lorem Ipsum...'
Instead of:
u('Lorem Ipsum')->truncate(8, '…', false); // 'Lorem Ipsum'

Thanks to @jmsche for his help.

@@ -1450,6 +1450,7 @@ public static function provideTruncate()
['foobar...', 'foobar foo', 6, '...', false],
Copy link
Contributor

@apfelbox apfelbox Jun 2, 2020

Choose a reason for hiding this comment

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

Sort-of unrelated, but what happens with ['???', 'foobar', 5, '...'],
ie. if ($length + $ellipsisLength) > $stringLength?
It should return the full string then, possibly?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what you mean, please provide a test case if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was not sure whether the length provided to the method is including or excluding the ellipsis. But apparently it is, so the case I talked about can't happen here.

So nvm then 👍

@nicolas-grekas nicolas-grekas added this to the 5.1 milestone Jun 4, 2020
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I updated the implementation to make more local, good catch overall.

@@ -1450,6 +1450,7 @@ public static function provideTruncate()
['foobar...', 'foobar foo', 6, '...', false],
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what you mean, please provide a test case if you want.

@nicolas-grekas
Copy link
Member

Thank you @duboiss.

@nicolas-grekas nicolas-grekas merged commit 69c37c0 into symfony:5.1 Jun 11, 2020
@fabpot fabpot mentioned this pull request Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants