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

Add Str::start() and str_start helper function #20569

Merged
merged 1 commit into from
Aug 15, 2017

Conversation

calebporzio
Copy link
Contributor

What's life if you can't PR string helper functions - amiright?

With some naming help from twitter and @adamwathan, I planned on PRing str_prefix and str_suffix (Those were the helper functions @DanielCoulbourne and I talked about on the podcast) as a cure for the common and gnarly looking ltrim($path, '/').'/' and rtrim($path, '/').'/', but upon writing up tests discovered str_finish face palm

I often need this sort of thing when dealing with urls and paths. Hopefully the need is felt.

Long live the string helper!

@taylorotwell taylorotwell merged commit a9ed75c into laravel:5.4 Aug 15, 2017
@taylorotwell
Copy link
Member

I accept your string helper offering.

@DanielCoulbourne
Copy link
Contributor

Proud of you bud.

@adamwathan
Copy link
Contributor

adamwathan commented Aug 15, 2017

Should benchmark the regex implementation in this and finish; makes sense to replicate the finish implementation but I can't help but think it's total overkill in both situations vs. ltrim and rtrim.

If the code was significantly simpler with the regex I wouldn't even say anything, but actually feels more complicated than this:

return $prefix.ltrim($value, $prefix);

Maybe there's some other reason it needs to be fancy though? Entirely possible!

(FWIW I would have PR'd it the exact same way you did, to be clear 😄)

@wilburpowery
Copy link

The helper man. Congrats @calebporzio 👌🏻

@vlakoff
Copy link
Contributor

vlakoff commented Aug 16, 2017

@adamwathan The trim methods work on single chars (the string is a "mask").

e.g. trim('foobar', 'afr') returns oob

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.

6 participants