WordPress.org

Make WordPress Core

Opened 18 months ago

Last modified 8 months ago

#22300 new enhancement

enhance urlencode_deep()

Reported by: wpmuguru Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: General Keywords: has-patch
Focuses: Cc:

Description

It would be handy if urlencode_deep() handled objects as well as arrays. Patch includes corresponding urldecode_deep()

Attachments (3)

22300.diff (2.5 KB) - added by wpmuguru 18 months ago.
22300.2.diff (2.5 KB) - added by wpmuguru 18 months ago.
remove the unneeded {/} in 22300.diff
map_deep.diff (3.5 KB) - added by nbachiyski 18 months ago.

Download all attachments as: .zip

Change History (18)

wpmuguru18 months ago

comment:1 scribu18 months ago

You don't really need the squigly braces there: $value->$property.

wpmuguru18 months ago

remove the unneeded {/} in 22300.diff

comment:2 wpmuguru18 months ago

Can someone remove the extra patch file? The .2 and .2.2 are the same file.

comment:3 nbachiyski18 months ago

Thanks for the patch, wpmuguru!

The two functions look really similar to me. I think we should have a common map_deep() and these two will just return map_deep( 'urldecode', $value) and map_deep( 'rawurlencode', $value ).

comment:4 nacin18 months ago

Something like map_deep(), while it could have a few other use cases in core, could really slow down the deep mapping it is trying to do.

comment:5 nbachiyski18 months ago

Actually, in my testing map_deep() is around twice faster for me for non-literal string values. I have no explanation.

Here is a very lame and crude testing script:

https://gist.github.com/ac7c4d5f783880917758

I tried with op-code cache on and off, also I tried swapping the order of running the urldecode_deep/map_deep, I also tried using different functions, not only urldecode().

There is a chance I did some stupid mistake in the script, so I'd love a second look.

comment:6 scribu18 months ago

I get PHP Warning: urldecode() expects parameter 1 to be string, array given in /home/scribu/Desktop/bench.php on line 19 when running bench-map-deep.php

comment:7 nbachiyski18 months ago

This wasn't affecting how the script worked. I fixed it, though.

comment:8 wpmuguru18 months ago

I reversed the order in which the loops are done (rawurldecode_deep first) & the loop executed first is still about 3 times faster than the second loop.

I do think the map_deep function does have merit though as WP has a few other *_deep functions. map_deep would be something plugin & theme devs would be able to use.

comment:9 nbachiyski18 months ago

It turned out that the idea of running them sequentially wasn't so good. I tested some more and the final result was that there is almost zero difference. Which means we should go ahead with the map_deep().

Here is a patch for the introduction of map_deep() and converting all older *_deep() functions to using it.

All the old tests pass and here are some for map_deep().

nbachiyski18 months ago

comment:10 scribu18 months ago

  • Milestone changed from Awaiting Review to Future Release

+1 for map_deep().

comment:11 wonderboymusic18 months ago

Every time I see this, I think is says "mad deep"

comment:12 nbachiyski18 months ago

The path from map_deep() to "mad deep", to "truly, madly, deeply" isn't very long.

Last edited 18 months ago by nbachiyski (previous) (diff)

comment:13 wpmuguru18 months ago

map_deep.diff looks good.

comment:14 SergeyBiryukov17 months ago

  • Version changed from trunk to 3.4

comment:15 jdgrimes8 months ago

+1 for map_deep(). Patch is still applying cleanly.

Note: See TracTickets for help on using tickets.