New truncate and truncateWords methods #2942

Merged
merged 8 commits into from Apr 29, 2014

Conversation

Projects
None yet
8 participants
@Alex-Code
Contributor

Alex-Code commented Apr 1, 2014

New truncate and truncateWords methods, resolves #2941

New truncate and truncateWords methods
New truncate and truncateWords methods, #2941
@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Apr 1, 2014

Member

Would be great to get some unit tests for edge cases.

Member

samdark commented Apr 1, 2014

Would be great to get some unit tests for edge cases.

framework/helpers/BaseStringHelper.php
+ */
+ public static function truncate($string, $length, $suffix = '...')
+ {
+ return substr($string, 0, $length) . $suffix;

This comment has been minimized.

@mikehaertl

mikehaertl Apr 1, 2014

Contributor

$suffix should only get appended, if the string was really truncated.

@mikehaertl

mikehaertl Apr 1, 2014

Contributor

$suffix should only get appended, if the string was really truncated.

framework/helpers/BaseStringHelper.php
+ {
+ $words = preg_split('/(\s+)/', $string, null, PREG_SPLIT_DELIM_CAPTURE);
+
+ return implode('', array_slice($words, 0, ($count * 2) - 1)) . $suffix;

This comment has been minimized.

@mikehaertl

mikehaertl Apr 1, 2014

Contributor

Same here: Only append suffix if the string has more words than $length.

@mikehaertl

mikehaertl Apr 1, 2014

Contributor

Same here: Only append suffix if the string has more words than $length.

framework/helpers/BaseStringHelper.php
+ public static function truncate($string, $length, $suffix = '...')
+ {
+ if (strlen($string) > $length) {
+ return substr($string, 0, $length) . $suffix;

This comment has been minimized.

@cebe

cebe Apr 1, 2014

Member

please use mb_strlen and mb_substr here to support multibyte strings.

@cebe

cebe Apr 1, 2014

Member

please use mb_strlen and mb_substr here to support multibyte strings.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Apr 1, 2014

Member

We also need a line in the CHANGELOG file.

Member

cebe commented Apr 1, 2014

We also need a line in the CHANGELOG file.

@cebe cebe added the type:feature label Apr 1, 2014

@Sammaye

This comment has been minimized.

Show comment
Hide comment
@Sammaye

Sammaye Apr 1, 2014

I did something similar to truncateWord but it truncates based on letter count to word count:

public static function truncateByWord($string, $maxCharLength = 100, $append = '...')
{
    $string = strip_tags($string);
    $words = preg_split('/\s+/', $string);

    $reformed_string = '';
    for($i = 0, $size = sizeof($words); $i < $size; $i++){
        if(strlen($reformed_string.' '.$words[$i]) > $maxCharLength){
            if($append){
                $reformed_string .= $append;
            }
            break;
        }else{
            if($i > 0){
                $reformed_string .= (' '.$words[$i]);
            }else{
                $reformed_string .= $words[$i];
            }
        }
    }
    return $reformed_string;
}

It's a bit sloppy but if you want to use it feel free to add it.

Sammaye commented Apr 1, 2014

I did something similar to truncateWord but it truncates based on letter count to word count:

public static function truncateByWord($string, $maxCharLength = 100, $append = '...')
{
    $string = strip_tags($string);
    $words = preg_split('/\s+/', $string);

    $reformed_string = '';
    for($i = 0, $size = sizeof($words); $i < $size; $i++){
        if(strlen($reformed_string.' '.$words[$i]) > $maxCharLength){
            if($append){
                $reformed_string .= $append;
            }
            break;
        }else{
            if($i > 0){
                $reformed_string .= (' '.$words[$i]);
            }else{
                $reformed_string .= $words[$i];
            }
        }
    }
    return $reformed_string;
}

It's a bit sloppy but if you want to use it feel free to add it.

Test case for new truncate methods.
Test case for new truncate methods.
framework/helpers/BaseStringHelper.php
+ * @param string $suffix A value to affix to the end.
+ * @return string the truncated string.
+ */
+ public static function truncate($string, $length, $suffix = '...')

This comment has been minimized.

@vova07

vova07 Apr 6, 2014

Contributor

Must be added an extra param $encoding = 'UTF-8' by default. It must be used in return line: return trim(mb_substr($string, 0, $length, $encoding)) . $suffix;

@vova07

vova07 Apr 6, 2014

Contributor

Must be added an extra param $encoding = 'UTF-8' by default. It must be used in return line: return trim(mb_substr($string, 0, $length, $encoding)) . $suffix;

This comment has been minimized.

@cebe

cebe Apr 6, 2014

Member

encoding can be used from Yii::$app->encoding

@cebe

cebe Apr 6, 2014

Member

encoding can be used from Yii::$app->encoding

+ $this->assertEquals('this is a string...', StringHelper::truncateWords('this is a string test', 4));
+ $this->assertEquals('this is a string!!!', StringHelper::truncateWords('this is a string test', 4, '!!!'));
+ $this->assertEquals('this is a big space...', StringHelper::truncateWords('this is a big space string', 5));
+ }
}

This comment has been minimized.

@xwz

xwz Apr 9, 2014

You need to include test cases for non-latin characters. E.g. CJK, arabic, etc.

@xwz

xwz Apr 9, 2014

You need to include test cases for non-latin characters. E.g. CJK, arabic, etc.

This comment has been minimized.

@Sammaye

Sammaye Apr 9, 2014

Problems could arise with Arabic, not all words are separated by a space.

@Sammaye

Sammaye Apr 9, 2014

Problems could arise with Arabic, not all words are separated by a space.

@qiangxue qiangxue added this to the 2.0 RC milestone Apr 16, 2014

framework/helpers/BaseStringHelper.php
+ */
+ public static function truncate($string, $length, $suffix = '...', $encoding = null)
+ {
+ if (mb_strlen($string) > $length) {

This comment has been minimized.

@samdark

samdark Apr 18, 2014

Member

Encoding isn't specified. Could lead to corrupted strings.

@samdark

samdark Apr 18, 2014

Member

Encoding isn't specified. Could lead to corrupted strings.

This comment has been minimized.

@Alex-Code

Alex-Code Apr 22, 2014

Contributor

If encoding is null it defaults to the application charset?

@Alex-Code

Alex-Code Apr 22, 2014

Contributor

If encoding is null it defaults to the application charset?

This comment has been minimized.

@samdark

samdark Apr 22, 2014

Member

Yes, same as in line 102.

@samdark

samdark Apr 22, 2014

Member

Yes, same as in line 102.

This comment has been minimized.

@Alex-Code

Alex-Code Apr 22, 2014

Contributor

Sorry, It's early here. What's the issue?

@Alex-Code

Alex-Code Apr 22, 2014

Contributor

Sorry, It's early here. What's the issue?

This comment has been minimized.

@samdark

samdark Apr 22, 2014

Member

The issue is that currently it does mb_strlen w/o specifying encoding. It should be as in line 102.

@samdark

samdark Apr 22, 2014

Member

The issue is that currently it does mb_strlen w/o specifying encoding. It should be as in line 102.

This comment has been minimized.

@Alex-Code

Alex-Code Apr 22, 2014

Contributor

See it now, Updated :)

@Alex-Code

Alex-Code Apr 22, 2014

Contributor

See it now, Updated :)

framework/helpers/BaseStringHelper.php
+ */
+ public static function truncateWords($string, $count, $suffix = '...')
+ {
+ $words = preg_split('/(\s+)/', trim($string), null, PREG_SPLIT_DELIM_CAPTURE);

This comment has been minimized.

@samdark

samdark Apr 18, 2014

Member

Regex should be with unicode modifier.

@samdark

samdark Apr 18, 2014

Member

Regex should be with unicode modifier.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Apr 18, 2014

Member

@Alex-Code could you look into encoding stuff here? Thanks.

Member

samdark commented Apr 18, 2014

@Alex-Code could you look into encoding stuff here? Thanks.

@samdark samdark self-assigned this Apr 22, 2014

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Apr 22, 2014

Member

One more thing. Changelog is still for beta. I guess you need to update the branch.

Member

samdark commented Apr 22, 2014

One more thing. Changelog is still for beta. I guess you need to update the branch.

@samdark samdark merged commit 46f381d into yiisoft:master Apr 29, 2014

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Apr 29, 2014

Member

Merged with some adjustments. Thanks for working on it.

Member

samdark commented Apr 29, 2014

Merged with some adjustments. Thanks for working on it.

@Alex-Code Alex-Code deleted the Alex-Code:stringhelper-truncate branch Apr 29, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment