docs for MenuLinkTreeInterface/menu_build_tree() unclear

To clarify what is going on:

a) 'expanded':

The value you pass in is an array of "parent link IDs", or "plids". This causes the function to only return a tree of menu items whose parent items have IDs that are in this list. If you do not supply 'expanded' (yes, it is optional), then the whole tree is returned (subject to the other restrictions in $parameters of course).

b) 'only_active_trail': I don't see the comma splice here? The "if" is a dependent clause, with a conjunction... it's not a comma splice but I agree that the comma should be removed.

c) optional keys: Actually, all of the parameter keys are optional -- in fact, $parameters itself is marked (optional), so I think it is implied that every one of the keys is optional? I don't think we need to mark each one.

So... What I think we need to do here is:

1. Fix 'expanded' docs so they make sense
2. Remove the comma in the second sentence in 'only_active_trail'.
3. Change @see in _menu_build_tree to a sentence saying "See menu_build_tree() for an explanation of the parameters."

Seems like a good Novice project?

Let's please wait with all this menu.inc docs when is in #2207893: Convert menu tree building to a service. Yes this is egoistic but these kind of reroles are painfull as the patchfile removes all the lines, so it is really hard to detect what changed in the meantime.

That is not egoistic, merely practical (since that other issue is probably higher priority). So yes, let's wait to commit this until that other issue is taken care of. (At which point, this issue may become "D7 only".)

Documentation updated as per suggestions. Regarding suggestion 3 (in comment #1), I'm not sure whether it would have been worthwhile to also retain the comment "@see menu_build_tree()".

Thanks for the patch! A couple of things will need to be fixed:

a) There are a lot of extra spaces in this patch. Please only leave one space after a . in documentation paragraphs.

b) This is not accurate:

+ *  - expanded: An array of parent link ids, or 'plids'.  The function returns
+ *    a tree of children items of the plids that are passed in.  If the
+ *    'expanded' key is not supplied the whole menu tree is returned, unless
+ *    the 'only_active_trail' key is passed in as TRUE.

There are many other conditions in $parameters that could result in not getting the entire menu tree, such as 'depth', so I would not be specific here about saying the whole menu tree is returned... this just needs to be reworded something like "... to restrict the tree to child items of the ..."

And a couple of notes:

1. Yes, you could leave in the @see if you want to.

2. We really do need to wait on this issue until that other one is resolved (see #3), so I am marking it postponed. Please do not bother making a patch here until that other issue is resolved, because it will most likely need to be rerolled.

#2207893: Convert menu tree building to a service. is in, so looks like it can be unpostponed.

OK. So menu_build_tree() in Drupal 8 is gone. Now we have
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Menu!MenuLinkTree...
which goes to a service. So ... someone needs to read through the methods on that interface and see if they are clearly documented or not. If that interface has the same problems identified here for the old function, let's fix it on this issue. Otherwise, this issue can be moved to D7.

I looked through MenuLinkTreeInterface:

a) The whole thing needs some grammatical/clarity editing. Also, paragraphs need to be separated by a blank line; otherwise they are not considered separate paragraphs and they should be wrapped as one paragraph.

b) The doc block at the top and several of the methods: @see lines should be at the end, not in the middle. But since these two @see lines are really intended to go with the preceding paragraphs, they should be changed to text similar to this and added to the paragraphs: See \Drupal\Core\Class\Name\Goes\Here for more information about blah blah blah.

c) Our current docs standards do not allow you to do ::methodName(). Prefix by the class name.

d) In the getCurrentRouteMenuTreeParameters() method, list syntax -- list items should start with a capital letter and end with .

... I got tired of listing the problems. Here, instead, is a patch for Drupal 8.

So... maybe this is out of scope, but this interface is the replacement for the menu tree functions of Drupal 7. The problem identified in this D7 issue doesn't really exist any more, because the menu tree build parameters have been replace by a parameters structure. ... it had some problems too...

This patch still applies. Anyone want to review it please?

I looked over the code. Verified that it still applies nicely (and works). This looks good to go.

Committed e41f90f and pushed to 8.0.x. Thanks!

I don't think we should backport this issue to D7 since MenuLinkTreeInterface does not exist there.

The original issue was about menu_build_tree(), which has/had the same docs.

Tagging the Drupal 7 backport of this patch for Drupalcon Dublin.