canton7 / fuelphp-casset

Better asset management library for fuelphp (with minification!)
MIT License
103 stars 29 forks source link

Change file_put_contents to use File::update #16

Closed dmyers closed 12 years ago

dmyers commented 12 years ago

In the combine() function I think its good to use Fuel's built in File class to do file changes.

What do you think?

canton7 commented 12 years ago

If we were going to move over to using Fuel's File class over native PHP stuff, I'd want to move everything over (including all of the file_get_contents calls), rather than just that one call to file_put_contents.

However, there's an issue in that file_get_contents can handle remote files (over http, for example), while Fuel's File class cannot. The docs say that, given a certain set of configuration, we'll go and fetch the remote file and cache it locally. Since the bit of the code that reads the remote file is also used for reading local files, that implies that, until Fuel's File class supports remote files, we can't switch over to it.

If you've got a solution to the above problem, please go ahead!

dmyers commented 12 years ago

Oh I didn't realize that. I mainly changed it to figure out some permission issues I was having that Fuel's would tell me it couldn't write a file or something, but I'm not sure what to do. I really like your package by the way. :)

canton7 commented 12 years ago

What error exactly were you getting? There's always scope for improvement in Casset's error-handing.

dmyers commented 11 years ago

My app didn't have write permissions on my cache folder and the file_put_contents() line logs an error, but Fuel's checks if the file is writable and throws an exception which is nice for debugging.