WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 11 months ago

#11338 closed defect (bug) (fixed)

Custom Walker initialize error

Reported by: ShaneF Owned by: DrewAPicture
Milestone: 4.0 Priority: normal
Severity: normal Version: 2.9
Component: Posts, Post Types Keywords: good-first-bug has-patch
Focuses: docs, template Cc:

Description

While I was delvoping a customer walker for my wp_list_pages, I was having trouble getting it to read my customer extended class of Walker_Page. I was talking with DD32 last night and he came up with the solution..

wp_list_pages(array('walker' => 'new Walker_Page_Rabbit', 'title_li' => '', 'depth' => '1')); 

...instead of...

wp_list_pages(array('walker' => 'Walker_Page_Rabbit', 'title_li' => '', 'depth' => '1')); 

Clearly the reason for including the "new" was because in the 'category-template.php' and 'post-template.php' had...

 	if ( empty($r['walker']) )
 		$walker = new Walker_Page;
 	else
		$walker = $r['walker'];

I found out that having 'new' in the

$r['walker']

..caused a mismatched argument in the...

return call_user_func_array(array(&$walker, 'walk'), $args);

'new' would be passed on. Once I changed the code in the 'core' to include the new and I would define just 'Walker_Page_Rabbit' in the 'wp_list_page' and it worked perfectly.

 	if ( empty($r['walker']) )
 		$walker = new Walker_Page;
 	else
		$walker = new $r['walker'];

This is a major blocker for any people who are working on customer walkers. Attached is a patch that does work.

Attachments (4)

walker_patch.diff (1.8 KB) - added by ShaneF 5 years ago.
added Walker_Comment fix
11338.patch (2.7 KB) - added by garza 11 months ago.
documentation for wp_list_pages in post-template.php
11338.1.patch (4.2 KB) - added by garza 11 months ago.
Try two for the patch after feedback
11338.2.patch (2.8 KB) - added by DrewAPicture 11 months ago.

Download all attachments as: .zip

Change History (26)

@ShaneF5 years ago

added Walker_Comment fix

comment:1 @dd325 years ago

I'm positive that you're not supposed to pass a string to that function to start with.

Example code:

$walker = new Walker_Page_Rabbit();
wp_list_pages(array('walker' => $walker, 'title_li' => '', 'depth' => '1')); 

comment:2 @ShaneF5 years ago

  • Keywords ne added
  • Resolution set to wontfix
  • Status changed from new to closed

After doing some speed tests.. I can find no fault in this logic.

comment:3 @ShaneF5 years ago

  • Keywords needs-patch added; has-patch ne removed
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Nevermind. There is an error. I now have this:

$walker_category = new Walker_Category_Rabbit;
		// array('title_li' => '', 'orderby' => 'name', 'show_count' => 1, 'feed' => 'RSS', 'depth' => -1, 'echo' => 0, 'walker' => $walker_category)
		$category_args = array('title_li=&orderby=name&show_count=1&feed=RSS&depth=-1&echo=0&walker='.$walker_category);

and it kills the blog. I am figuring out what is going on.


comment:4 follow-up: @dd325 years ago

and it kills the blog. I am figuring out what is going on.

Because its invalid PHP.

for a start: $walker_category = new Walker_Category_Rabbit() - parenthesis on the end.

Secondly, You cannot convert a object to a string and expect it to work - Only use the array format if at all possible, infact, only the array format will work when passing objects.

comment:5 in reply to: ↑ 4 ; follow-up: @filosofo5 years ago

Replying to dd32:

Because its invalid PHP.

for a start: $walker_category = new Walker_Category_Rabbit() - parenthesis on the end.

I'm pretty sure parentheses are optional, although I can't find any official documentation that says one way or the other. I've never seen any syntax warnings when omitting them, most PHP books I've read omit them frequently, and even this PHP manual example of object instantiation omits them.

However, I'd certainly be interested in seeing any contradicting documentation you can point me to.

comment:6 in reply to: ↑ 5 @Denis-de-Bernardy5 years ago

Replying to filosofo:

However, I'd certainly be interested in seeing any contradicting documentation you can point me to.

parenthesis are optional here, if you don't pass any arguments to the constructor.

comment:7 @ryan5 years ago

If you new the walker before passing it and always use the array format, all is well, yes? That was the intention. Perhaps the phpdoc could be clarified.

comment:8 @dd325 years ago

I'm pretty sure parentheses are optional

Yep you're right.. Never knew that. Just assumed it since something was fatal erroring that was it..

ryan: perhaps the phpdoc should be clarified for it.. I wish people wouldnt use the string calling methods :P

comment:9 @westi5 years ago

  • Keywords needs-phpdoc added
  • Milestone changed from 2.9 to 3.0
  • Priority changed from high to normal
  • Severity changed from major to normal
  • Type changed from defect (bug) to enhancement

It looks like that wp_list_pages could do with some good phpdoc and then all would be fine.

I think we can move this out to 3.0 and get some documentation written and committed then.

comment:10 @ShaneF5 years ago

Rgr that.

comment:11 @ShaneF5 years ago

  • Summary changed from Customer Walker initialize error to Custom Walker initialize error

comment:12 @nacin5 years ago

  • Component changed from Formatting to Inline Docs

Anyone want to do some inline documentation for this?

comment:13 @ryan5 years ago

  • Milestone changed from 3.0 to 3.1

comment:14 @nacin4 years ago

  • Milestone changed from Awaiting Triage to Future Release

comment:15 @DrewAPicture18 months ago

  • Keywords needs-phpdoc removed

comment:16 @nacin14 months ago

  • Component changed from Inline Docs to Template
  • Focuses docs added

comment:17 @nacin14 months ago

  • Component changed from Template to General
  • Focuses template added

comment:18 @DrewAPicture11 months ago

  • Keywords good-first-bug added
  • Type changed from enhancement to defect (bug)

All of the default arguments for wp_list_pages() need documenting at some point, but I'll take just a description for the walker argument here if somebody wants to write one.

@garza11 months ago

documentation for wp_list_pages in post-template.php

comment:19 @garza11 months ago

  • Keywords has-patch added; needs-patch removed

Hi! Added some updated inline documentation for wp_list_pages in post-template.php, thanks to wcatx contributor day!

comment:20 @DrewAPicture11 months ago

  • Component changed from General to Posts, Post Types
  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Future Release to 4.0

Hi garza, thanks for the patch, this is a great start.

Two things:

  • Where the arguments are currently listed in quotes, e.g. 'child_of, they should be notated as variables instead: $child_of.
  • The purpose of the hash notation is both to spell out what the default arguments mean, as well as to define what the actual default values are. So you also need to list the default at the end of the descriptions, and the values they accept if only certain values are accepted.

Take a look at the PHPDoc block for wp_insert_post to see what I mean. Thanks again!

Last edited 11 months ago by DrewAPicture (previous) (diff)

@garza11 months ago

Try two for the patch after feedback

comment:21 @garza11 months ago

  • Keywords has-patch added; needs-patch removed

Attached 11338.1.patch after feedback from DrewAPicture, I tried to use the default description and acceptable values from the codex as much as possible, let me know if it's too much or not consistent.

@DrewAPicture11 months ago

comment:22 @DrewAPicture11 months ago

  • Owner set to DrewAPicture
  • Resolution set to fixed
  • Status changed from reopened to closed

In 28357:

Add inline documentation of the default arguments for wp_list_pages().

Props garza for the initial patches.
Fixes #11338.

Note: See TracTickets for help on using tickets.