Skip to content

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

Merged
taylorotwell merged 1 commit into
laravel:5.4from
calebporzio:str-start-helper
Aug 15, 2017
Merged

Add Str::start() and str_start helper function#20569
taylorotwell merged 1 commit into
laravel:5.4from
calebporzio:str-start-helper

Conversation

@calebporzio

Copy link
Copy Markdown
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
Copy Markdown
Member

I accept your string helper offering.

@DanielCoulbourne

Copy link
Copy Markdown
Contributor

Proud of you bud.

@adamwathan

adamwathan commented Aug 15, 2017

Copy link
Copy Markdown
Contributor

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
Copy Markdown

The helper man. Congrats @calebporzio 👌🏻

@vlakoff

vlakoff commented Aug 16, 2017

Copy link
Copy Markdown
Contributor

@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