Implement proxy variables for curl_setopt (part 2)#360
Implement proxy variables for curl_setopt (part 2)#360nerzhul wants to merge 2 commits intosimplepie:masterfrom
Conversation
library/SimplePie.php
Outdated
There was a problem hiding this comment.
maybe prepend a set_ and make them better readable, like set_proxy_host, set_proxy_port, set_proxy_userpassword.
You also dont need to set a parameter null since its null anyways and the user will call it with something different if he calls it at all
library/SimplePie.php
Outdated
There was a problem hiding this comment.
I'd make all three of them private. No need to write setters if they are public anyways
|
You are right, i'm copying "public $force_fsockopen = false;" and doing the same way (which is stupid, @access private with a public variable). |
|
Fixed |
|
Looks good, except the File class is used twice in fetch_data. Might also need to look at how Sanitize.php and Locator.php use the File class. |
|
@nerzhul ive implemented that in the current news app master btw, but i have to wait for this to be merged in here |
|
Okay. I don't know why, it seems the project is dead, no ? No commit or pull done since a long time |
|
@nerzhul i thought a lot about forking the lib already but it seems to much for me to maintain it. ATM Im doing the news, notes owncloud apps, working on core appframework stuff (MVC + DI for devs), app dev documentation and scaffolding tools. |
|
@nerzhul please see owncloud-archive/news#512 |
|
I have also searched for an alternative, but haven't found anything as complete as simplepie. However, please, keep sending patches, because they can always be merged in personal repos... |
|
@ifsnop nope, unless theres an maintained fork theres no point for us to send patches. It just wastes our time and I'm sure there are alternatives |
|
Well, I try to merge them on my own fork. My own patches haven't been |
|
@Raydiation I don't want to miss out on patches because simplepie is not being maintained. It would be great to hear from any of the current maintainers right now, I'm sure they're all receiving these emails. If there's no response though, I propose we create a fork. I'm happy to help maintain it if you and @ifsnop are willing to help. |
|
@nerzhul, just to be sure, the only two branches I would need to merge to get proxy support are patch-1 and patch4... right? |
|
@ifsnop of course. Patch 2 and patch 3 were replaced by patch 4 |
|
Hi guys! I'm so sorry for not getting time to look at this. Too many projects and not enough time, alas. I like the idea behind the patch, but as a whole, the PHP community is (rightly) moving towards more modular projects. Along those lines, I'd like to eventually get rid of Along those lines, I'm thinking we close this, since it's reasonably easy to use one of the aforementioned libraries instead, and they are much better at handling HTTP than SP can ever be. Thoughts? :) |
|
Why don't you like curl/libcurl? Is well maintained, tested, and does a pretty good job. Also it is always available... |
cURL is fine, but it's not the easiest beast to work with. There are lots of intricacies involved with using it, and it's very easy to screw this up. I'd prefer using a well-tested library instead.
It's definitely not; there's a not-insignificant percentage of hosts that don't have it. |
|
See also See also #407 |
Added set_curl_options method to allow custom options Conflicts: library/SimplePie.php library/SimplePie/File.php Choose the generic approach of adding any parameter to cURL simplepie#407 Barnetik@51f9f08 instead of the approach focusing only on proxy aspects simplepie#360 simplepie#359 FreshRSS/FreshRSS@9aab83a
|
fixed by #407 |
Linked with #359