#22300 closed enhancement (fixed)
enhance urlencode_deep()
Reported by: | wpmuguru | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | 3.4 |
Component: | Formatting | Keywords: | has-patch has-unit-tests commit |
Focuses: | Cc: |
Description
It would be handy if urlencode_deep() handled objects as well as arrays. Patch includes corresponding urldecode_deep()
Attachments (8)
Change History (32)
#3
@
12 years 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 )
.
#4
@
12 years 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.
#5
@
12 years 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.
#6
@
12 years 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
#8
@
12 years 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.
#9
@
12 years 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().
#12
@
12 years ago
The path from map_deep()
to "mad deep", to "truly, madly, deeply" isn't very long.
#16
@
9 years ago
- Keywords needs-refresh added
- Milestone changed from Future Release to 4.4
- Owner set to chriscct7
- Status changed from new to assigned
#17
@
9 years ago
- Keywords needs-refresh removed
22300.3.diff is a refresh and passes the unit tests in 22300-tests.diff.
#18
@
9 years ago
- Keywords commit added
Added missing since in the docbloc on map_deep. Otherwise looks good
#19
@
9 years ago
- Component changed from General to Formatting
- Keywords needs-unit-tests added; commit removed
map_deep()
looks handy.
Let's get some unit tests on this (for example, by testing urlencode_deep()
which doesn't currently have any test coverage).
#20
@
9 years ago
- Owner changed from chriscct7 to johnbillion
@johnbillion see 22300.test1.diff for proposed tests
#21
@
9 years ago
- Keywords has-unit-tests commit added; needs-unit-tests removed
22300.4.diff changes the order of the parameters in map_deep()
so they match array functions in PHP such as array_walk_recursive()
. It also implements a tidied up version of the original tests for map_deep()
from 22300-tests.diff in addition to the tests from 22300.test1.diff, and improves docs.
This is good to go.
You don't really need the squigly braces there:
$value->$property
.