Skip to content
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

[5.4] Add str_after helper function #19357

Merged
merged 3 commits into from
May 28, 2017

Conversation

calebporzio
Copy link
Contributor

I often come across cases where I need to retrieve everything after a piece of a string. Manually preg_match or strpos / substr is verbose IMO.

Here is the usage:

$title = str_after('Title: hello world', 'Title: ');

I've been using my own personal helper for this for some time now and have found it extremely handy, hoping you find the same!

Thanks for the endless amounts of hard work!

  • Caleb

@devcircus
Copy link
Contributor

devcircus commented May 26, 2017

Very useful. He rarely merges helpers that aren't used in the framework itself, but 'fingers-crossed'

@joshmanders
Copy link
Contributor

I doubt the helper will get merged for the reason @devcircus said, but I wouldn't doubt the String::after function being merged.

@JosephSilber
Copy link
Member

@calebporzio
Copy link
Contributor Author

@JosephSilber strstr returns a string including the search term. my str_after does not include the searched string.

@taylorotwell
Copy link
Member

I can't decide what I think about returning empty string if the given string isn't present. To me it feels like I would want the whole string returned, since it most cases I would be using this function to trim a given string off the front of the other string. Thoughts?

@JosephSilber
Copy link
Member

JosephSilber commented May 26, 2017 via email

@taylorotwell
Copy link
Member

I agree there are existing options if it's a leading trim, this would just be a more readable version of it maybe. Anyways, probably need @calebporzio to comment with his thoughts behind it.

@tillkruss tillkruss changed the title Add str_after helper function [5.4] Add str_after helper function May 27, 2017
@calebporzio
Copy link
Contributor Author

I'm good with returning the entire string rather than empty. That makes more sense to me.

I thinkstr_after should not need the search string to be the start of the subject. A more fitting regex would be preg_replace('/^.*' . preg_quote($search) . '/')

I adjusted the implementation to return the full string on failure. I didn't go the regex route, let me know if that's a problem.

@sisve
Copy link
Contributor

sisve commented May 28, 2017

The docs for strpos

Returns FALSE if the needle was not found.

The docs for strstr ...

Returns the portion of string, or FALSE if needle is not found.

The docs for substr ...

Returns the extracted part of string; or FALSE on failure, or an empty string.

It kind of makes sense to keep the php tradition of returning false for these kind of string operations. Substr's empty string is a special case when pass $length = 0. If you you pass a $start which is after the length of the input you still get a false back.

If you want the original string if the needle is missing, use str_after($haystack, $needle) ?? $haystack (assuming PHP 7).

@taylorotwell taylorotwell merged commit 4c4810a into laravel:5.4 May 28, 2017
@vlakoff
Copy link
Contributor

vlakoff commented May 30, 2017

There is room for optimization in this code, and at least an unit test with empty search should be added, because strpos triggers a warning if needle is empty. Currently there is no error thanks to the above contains, but this one might get removed in the future.

return $subject;
}

$searchEndPos = strpos($subject, $search) + static::length($search);
Copy link
Contributor

Choose a reason for hiding this comment

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

strpos is single-byte while the rest of the code is utf8 aware.
Therefore you get wrong results with after('é foobarbaz', 'foo')


$searchEndPos = strpos($subject, $search) + static::length($search);

return static::substr($subject, $searchEndPos, static::length($subject));
Copy link
Contributor

Choose a reason for hiding this comment

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

We grab up to the end, so the third parameter should be omitted.
(as more, here it exceeds the end the subject; it works but is erroneous)

@BrandonShar
Copy link
Contributor

This seems unintuitive if there are multiple instances of the search string. I think I'd expect
str_after('Title: Title: hello world', 'Title: '); to return hello world, but it will return Title: hello world

@kingvish
Copy link

kingvish commented Jun 6, 2017

There can be third parameter offset like after($subject, $search, $offset) to skip number of character / occurrence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants