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

[Proposal] Add the old Laravel\Str class #69

Closed
maximebeaudoin opened this issue Jan 16, 2013 · 21 comments
Closed

[Proposal] Add the old Laravel\Str class #69

maximebeaudoin opened this issue Jan 16, 2013 · 21 comments

Comments

@maximebeaudoin
Copy link
Contributor

I'm trying to figuring out where is the old Laravel\Str but i don't find any reference to this class methods. There is any reason why this library is not available anymore? I think it will be a good idea to reintegrate this library in Laravel 4.

@yuters
Copy link
Contributor

yuters commented Jan 16, 2013

You can have a look here : https://github.com/meido

Hope it helps you out ;)

@maximebeaudoin
Copy link
Contributor Author

Nice thank you.

I wonder if Taylor plans against its inclusion in Laravel 4. If not, why ? :)

@Anahkiasen
Copy link
Contributor

Well now that Laravel is modular and that Str can be added to your application in one line he really doesn't have any reason to add it back.

@bencorlett
Copy link
Contributor

Yeah I think ditch the string class. It's way to application specific and there are whole projects out there (as was pointed out earlier) dedicated to making much better ones. Same goes for the asset class.

@robclancy
Copy link
Contributor

https://github.com/bigelephant/string is the L3 string class adapted to L4 coding standards and also things abstracted. String functions found in framework/support have also been added.

@Anahkiasen
Copy link
Contributor

There are actually three ports of Str on packagist, which is kind of weird :

https://github.com/meido/str
https://github.com/Anahkiasen/string
https://github.com/lembubintik/dictate

@robclancy
Copy link
Contributor

4*

@maximebeaudoin
Copy link
Contributor Author

I see !!! This is why i think this String Helper should be in the framework instead. But i'm agree with bencorlett too. To stay application specific only, this helpers should be out of the box :)

@helmut
Copy link
Contributor

helmut commented Jan 17, 2013

I think when functionality is useful for most aspects throughout the entire system it should be put in. Something as generic as applying functions to a string could be used right throughout the system. For example there are plenty of Filesystem abstraction classes out there but laravel has its own implementation baked in because it is used throughout.

As long as it is generally useful and easily extended or overloaded, I see no problems with giving access to this kind of functionality out of the box.

@robclancy
Copy link
Contributor

Not a good example, I find the Filesystem component very lacking. It feels rushed in.

My String component does feel like it should be something like what should come by default as I not only have all the methods from the L3 class but also the Support/helpers.php string methods which are obviously already used throughout laravel but as string_contains() etc instead of String::contains().

But then again, it really isn't hard to pick and choose a string component to use.

@bencorlett
Copy link
Contributor

The main reason the file system helper exists is because it allows abstraction of the server's file system, mainly for testing. You can mock what exists on the file system etc

Sent from my iPad
Please excuse my brevity

On 18/01/2013, at 2:57 PM, Robert Clancy [email protected] wrote:

Not a good example, I find the Filesystem component very lacking. It feels rushed in.

My String component does feel like it should be something like what should come by default as I not only have all the methods from the L3 class but also the Support/helpers.php string methods which are obviously already used throughout laravel but as string_contains() etc instead of String::contains().

But then again, it really isn't hard to pick and choose a string component to use.


Reply to this email directly or view it on GitHub.

@robclancy
Copy link
Contributor

Yes I know why it exists. I just said it was a poor component. It is barely abstracted as there isn't even an interface. And there are basics things something like that should do. I have already run into situations where I can't use it because it doesn't do what I needed to do so I will either make my own component in future to replace it or find another one.

@helmut
Copy link
Contributor

helmut commented Jan 18, 2013

It's not really about the Filesystem class. Just as an example of a component to perform generic tasks that is utilised throughout the entire framework.

@robclancy
Copy link
Contributor

Yeah I know. Taylor just added some more string functions to helper.php. So looks like everything is going to be in the form of str_* functions. I don't like this idea at all but I will be using my String component anyway.

@helmut
Copy link
Contributor

helmut commented Jan 20, 2013

It just seems that by moving these things out of a class it makes it harder to extend or override this functionality. If the framework didn't use any string functions to run then I would say leave it out. But moving them into a slightly hidden helper file into secret (or not documented) functions doesn't really help anyone trying to extend the framework at all. It's like laravel doesn't want you to have access to the same functionality that it uses for itself.

@taylorotwell
Copy link
Member

I'm going to do this, just haven't had time. Should be able to this week.

@maximebeaudoin
Copy link
Contributor Author

Great, I think it will be appreciated.

@johnnncodes
Copy link

Good to know that this will be added :)

@broucz
Copy link

broucz commented Jan 30, 2013

Good news to see it back ! tnx

@bencorlett
Copy link
Contributor

Haha @helmut, that made me laugh:

"It's like laravel doesn't want you to have access to the same functionality that it uses for itself."

Scandal!

@taylorotwell
Copy link
Member

I've added the class.

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

No branches or pull requests

9 participants