php-proxy icon indicating copy to clipboard operation
php-proxy copied to clipboard

Small cleanup

Open reef-actor opened this issue 6 years ago • 5 comments

'remove_js' & 'replace_title' didn't seem to belong in the ProxifyPlugin, so have been given their own plugins.

ProxifyPlugin has been reorganised, primary difference is the use of named capture groups in the RegEx's to allow reuse of the callback and therefore reduce duplicated code. Image and Font content has been added to the blacklist to reduce load.

AbstractPlugin has been simplified to directly subscribe to events, removing 2 extra function calls per event per plugin.

reef-actor avatar Jan 29 '18 11:01 reef-actor

I removed the Html class as it is no longer a dependency following the extraction of js_remove and inlining the RegEx. Since some of the plugins in php-proxy-plugin-bundle are dependent on it, it might make sense to merge it with the utils there before removing it here. Also, is Redis.php still needed in this project? Seems to be unused and superseded by php-proxy-plugin-cache?

reef-actor avatar Jan 31 '18 09:01 reef-actor

It looks like there is also a dependency on the url_pattern functionality from php-proxy-plugin-bundle so that goes back in.

reef-actor avatar Feb 01 '18 10:02 reef-actor

Interesting changes @reef-actor

@Athlon1600 what do you think about these changes?

I like the refactor of ProxifyPlugin and that remove_js and replace_title have becomed a plugin.

webaddicto avatar Feb 04 '18 19:02 webaddicto

It has occurred to me that const visibility modifiers were only introduced in php 7.1 so the use of private const should probably be changed to just const to avoid unnecessarily restricting compatibility.

reef-actor avatar Feb 04 '18 22:02 reef-actor

Some of those could probably go through. I will have more time to review this next week though... I'm looking right now to see what happened to Html class.

Athlon1600 avatar Feb 11 '18 23:02 Athlon1600