If #2044969: Break the old router for contrib is a go then it is now critical to document the router. There is zero documentation as far as I am concerned, the rough overview over at Symfony is inadequate even for them and doesn't cover Drupal at all. We need flowcharts, internals (so that one can debug it), explanations (why do we use _controller / _form instead of just form? etc) and examples.

For posterity, the documentation or some semblance of it now can be found at https://drupal.org/node/2122071 . It didn't even take a year to get some documentation up. And that, of course, was not written by anyone in the WSCCI team because they are exempt from such small details like producing developer friendly, performant, documented code.

Comments

Lets do it on drupal.org: https://drupal.org/node/2046371

Note: this was just a start and needs flowcharts etc.

Daniel - I can start working on helping with this issue by working from your draft and starting on some flowchart/summaries. Any guess on how much work/time is needed to address this issue? As it might help to set a target timeline timeline/overall scope for what's involved (and if needed actively recruit some others to pitch in). I'll be at MWDDS this w/end if you want to discuss.

- Willy

It is for sure hard to tell how much time it will need to document the router, especially as this depends on your knowledge of the router system.

I won't by at MWDDS but you can always contact me via IRC

I have a solid understanding of the D7 router system and experience implementing in custom modules. I've been learning the D8 router system and have been learning the Symfony routing system and understand/follow what's outlined here: https://drupal.org/node/1800686. So just wondering if you think I could pitch on this issue (at least with initial drafts) or defer to someone with more expertise? As mentioned, I could revised the draft that you've prepares and take a shot at some flowcharts.

Please go for it!

Agreed, that's fantastic! And it's always easier for people to review and correct documentation that exists than make it up to begin with. :)

Thanks for taking a stab, willyk!

Issue summary:View changes

Updated issue summary.

Component:routing system» documentation
Issue tags:+WSCCI

We also need actual API documentation in the codebase, ideally a @defgroup that includes various controller-related classes and describes the different _parameterthingies and such.

Moving to the correct component to get @jhodgdon's review once we have a patch.

Title:Document the routerMenu routing docs do not match what the code is doing
Category:task» bug

I ran into this last week when I was trying to update some code to Drupal 8. The API docs on the router at
https://api.drupal.org/api/drupal/core!includes!menu.inc/group/menu/8
have not been updated to 8, and hook_menu() docs do not mention the new routing.yml stuff at all. This really needs to be fixed and is really a docs bug that they weren't updated when the menu routing system was overhauled.

Well at least menu.inc/the group menu is certainly the wrong place as it is all about old code. We probably want a group called "Routing"

Category:bug» task

Category:task» bug
Priority:Critical» Major

xjm pointed out in another issue that bad docs are bugs, so compromising at major bug - there's no fatal error, data loss or security issue so not critical.

By that definition, should missing change notices be demoted to major tasks, then?

Well it depends on whether it should block the release or not, we (as in possibly you and me personally in irc) had the idea of making change notices critical so they'd block the release and all get done in time. Probably they should still do that so they ought to be critical.

I don't mind if major docs omissions are major bugs or critical tasks, but the critical bugs queue needs to reflect actual, functional bugs otherwise it's impossible to have an overview of what's there. I've personally moved about 8 issues out of it today that were in the wrong place.

Critical tasks is 'stuff we have to do by release' which more open to interpretation.

I guess it comes down to if the last 10 critical tasks were all change notices, would we release RC1, or wait for them to be done?

Good question. We should probably split this off into its own discussion. Did so here: #2078085: Determine if change notices should block an 8.0 RC?

I just spent a couple of hours head-desking because {params} in routing.yml that happen to be the name of an entity type get turned into entity loaders.

> Well at least menu.inc/the group menu is certainly the wrong place as it is all about old code. We probably want a group called "Routing"

That, or find a suitable entry point to the routing system for the bulk of the docs to live on. Eg, which class processes routing.yml files?

Either way, we'd need a @see link from hook_menu().

As for whether this should block release -- do we want developers to be able to start porting their D7 modules? If so, docs are an essential requirement.

Might help to cover this in small chunks.

Here's one: #2087107: document routing.yml parameter converters.

Does the routing system have a docs entry point yet?

I guess there's https://api.drupal.org/api/drupal/core!includes!menu.inc/group/menu/8 but that is looking fairly out of date. I suggest we rewrite that, and add a new 'routing' topic.

I mentioned this in #2106709: Remove legacy router backward compatibility layer but can we please pretty please put in a stopgap in the hook_menu() docblock that says "Hey you! Hands off that page callback! Use the routing system!" We can even reference the change notice if need be. Of course the sooner the in-code API docs are complete and correct the better, but I'd at least like to give people a clue.

StatusFileSize
new717 bytes

Something like this? :)

That patch and the previous ones should never have gone in without a docs update in hook_menu() in the first place. sigh.

There are some good docs fixes in #2044969-36: Break the old router for contrib that we should bring into this issue, now that that issue is closed as a duplicate.

Priority:Major» Critical
Issue tags:+8.x-alpha4

This is a release blocker. In fact, I'd love to get this in for alpha4.

Agreed with this blocking the release, but it's not a functional bug. Restoring state to the same as #11.

Category:bug» task
Status:Active» Needs review
StatusFileSize
new9.48 KB
PASSED: [[SimpleTest]]: [MySQL] 58,484 pass(es).
[ View ]

I think @catch meant to make it back to task as per #11
So here's the salient parts from #2044969: Break the old router for contrib

Just in general we should get that one in as fast as possible as it will conflict with basically all other menu_router related patches.

+++ b/core/modules/system/system.api.php
@@ -668,15 +620,11 @@ function hook_menu_get_item_alter(&$router_item, $path, $original_map) {
   $items['example/feed'] = array(
     'title' => 'Example RSS feed',
-    'page callback' => 'example_feed',
-    'access arguments' => array('access content'),
-    'type' => MENU_CALLBACK,
+    'route_name' => 'example.feed',
   );

If you are hard that this entry can be removed as it is.

Why are titles not removed from hook_menu if they are now in the route.yml?

Please add a support question instead ... hook_menu() still contains menu links, which do have a title.

Status:Needs review» Needs work

I started from scratch and reviewed the patch in #25.

Some things I think should be fixed: some small, some larger:

a) We use serial commas in Drupal docs. So,

+ * This hook enables modules to register links to be displayed in menus,
+ * as breadcrumbs and as local tasks.

should be a comma before "and".

b) The first line says this hook is for defining menu links and local tasks, but the text in (a) also mentions breadcrumbs. How about if we add breadcrumbs to the first line and get rid of the text in (a) completely? They would then be redundant. The first line could be "Define links for menus, local tasks, and breadcrumbs.".

c) The paragraph after this is incorrect in D8:

  *
  * hook_menu() implementations return an associative array whose keys define
  * paths and whose values are an associative array of properties for each
  * path. (The complete list of properties is in the return value section below.)

The keys do not define paths. They might *refer* to paths, but they don't define them.

d) Our API docs for Drupal 8 should not refer back to previous versions. So this paragraph should just be removed:

* @section sec_callback_funcs Callback Functions
+ * The definition for each path may no longer include a page callback function.
+ * Registering page callbacks with hook_menu is not supported in Drupal 8.
+ * Instead you must create a module.routing.yml file which registers controllers
+ * for your module paths. See @link https://drupal.org/node/1800686 @endlink for
+ * further details.

And probably the next one too -- it's really not relevant for this hook to describe what used to be a function of hook_menu() in previous versions of Drupal. We need a separate topic for this, but it shouldn't be part of the hook_menu() docs.

+ * @section registering_menu_links Registering menu links
+ * For example your module.routing.yml could register path 'abc/def' as abc_def,
+ * to create a menu link and provide a page or breadcrumb title for this link
+ * you would add the following to your hook_menu() definition.
+ * @code
+ *   function mymodule_menu() {
+ *     $items['abc/def'] = array(
+ *       'title' => 'View abc for def',
+ *       'route_name' => 'abc_def',
+ *     );
+ *     return $items;
+ *   }
+ * @endcode
+ *

e) Actually, since the title of this issue is about menu routing docs, and not about hook_menu() we should probably add the separate topic about routing here and then we can put an @see into hook_menu(). Then this can be added to the meta-issue about making sure we have topics for new stuff in 8:
#2106873: [meta][policy, no patch] Make Drupal 8 developer docs more discoverable

f) The stuff about path component substitution seems to not exist in hook_menu() in D8 right now. However, some of the lines left in the hook docs refer to it. These need to be rewritten.

Just throwing this out there. We have an alpha release happening on Friday. Is it possible we could put the bulk of this in HEAD before then to address the critical issue of "this stuff is just flat-out wrong," but leave it open as a "major" to address further clean-up?

I'll make a new patch. I still think the current patch has too much incorrect stuff in it.

Status:Needs work» Needs review
StatusFileSize
new12.36 KB
PASSED: [[SimpleTest]]: [MySQL] 59,223 pass(es).
[ View ]

How about this as a quick stopgap? I added a couple of @todos... also started modifying the Menu topic that was also totally wrong.

StatusFileSize
new5.57 KB

forgot interdiff

Nice work!

@@ -19,17 +19,36 @@
+ * Once you have a route defined, you can use hook_menu() to define links
+ * and breadcrumbs for your module's paths in the main Navigation menu or other
+ * menus. See the hook_menu() documentation for more details.

I think we shouldn't suggest using hook_menu() to define links but the substitutions in development, like hook_default_menu_links() and BreadcrumbBuilderInterface implementation.

I don't think we can suggest using hooks that will not be in the alpha. We could put a @todo there saying that hook_menu() will most likely be replaced, and clue people in that it's still to early to port modules.

StatusFileSize
new546 bytes
new12.51 KB
PASSED: [[SimpleTest]]: [MySQL] 59,509 pass(es).
[ View ]

Here, with a @todo alert about hook_menu().

I'm agree jhodgon, and the @todo is a good point to avoid confusion with hook_menu()

  1. +++ b/core/includes/menu.inc
    @@ -19,17 +19,38 @@
    + * Once you have a route defined, you can use hook_menu() to define links
    + * and breadcrumbs for your module's paths in the main Navigation menu or other
    + * menus. See the hook_menu() documentation for more details.

    The bit about breadcrumbs is not true anymore. Breadcrumbs are handled 100% via the routing system and paths.

  2. +++ b/core/modules/system/system.api.php
    @@ -491,21 +490,10 @@ function hook_menu_get_item_alter(&$router_item, $path, $original_map) {
    + * Define links for menus, breadcrumbs, and local tasks.

    Let's keep it to only define menu links, every other subsystem is thrown out/ will be thrown out.

  3. +++ b/core/modules/system/system.api.php
    @@ -517,25 +505,27 @@ function hook_menu_get_item_alter(&$router_item, $path, $original_map) {
      * $items['admin/config/system/foo/tab1'] = array(
      *   'title' => 'Tab 1',
      *   'type' => MENU_DEFAULT_LOCAL_TASK,
    - *   // Access callback, page callback, and theme callback will be inherited
    - *   // from 'admin/config/system/foo', if not specified here to override.
    + *   // Route name would be inherited from 'admin/config/system/foo', if not
    + *   // specified here to override.
      * );
      * // Make an additional tab called "Tab 2" on "Foo settings"
      * $items['admin/config/system/foo/tab2'] = array(
      *   'title' => 'Tab 2',
      *   'type' => MENU_LOCAL_TASK,
    - *   // Page callback and theme callback will be inherited from
    - *   // 'admin/config/system/foo', if not specified here to override.
    - *   // Need to add access callback or access arguments.
    + *   'route_name' => 'foo2',
      * );

    Let's skip those.

  4. +++ b/core/modules/system/system.api.php
    @@ -645,8 +574,6 @@ function hook_menu_get_item_alter(&$router_item, $path, $original_map) {
      *     - MENU_LOCAL_ACTION: Local actions are menu items that describe actions

    all the other items can be removed as well.

Status:Needs review» Needs work

dawehner or someone else: Can you make a new patch? I don't have time today and also I don't really know the menu/routing system as well as you do, obviously. I was just basing this on the previous patches and guesswork. Status based on last comment.

Status:Needs work» Needs review
StatusFileSize
new5.26 KB
new14.71 KB
PASSED: [[SimpleTest]]: [MySQL] 59,359 pass(es).
[ View ]

Some improvements with the previous suggestions.

I think this version is fine. Any other thoughts?

Status:Needs review» Reviewed & tested by the community

+++ b/core/includes/menu.inc
@@ -19,17 +19,38 @@
+ * @todo The rest of this topic has not been reviewed or updated for Drupal 8.x
+ *   and is not correct!

:)

---

All of the added docs are 100% correct, and the removals are accurate as well.

Priority:Critical» Major
Status:Reviewed & tested by the community» Active

Ok, excellent. Thank you SO MUCH all for all your help on this, everyone!

I'm going to go ahead and commit this, since it is obviously light years better than what is currently there. It sounds like we need to keep this open a bit longer though to resolve that @todo?

Committed and pushed to 8.x. Thanks!

Moving back to active and major. Or, feel free to close and re-open a separate, more focused issue that's not 50 replies long. :)

Status:Active» Postponed
Issue tags:+revisit before beta

It sounds like we should actually postpone this until the hook that is going to be used for menu links is stabilized? There is little point in writing docs that will just have to be rewritten.

Good call!

This made it into alpha4, so untagging. :)

Issue tags:+revisit before beta

Uh. Not that.

Issue summary:View changes

Remove unhelpful sniping.

Title:Menu routing docs do not match what the code is doingThe router is still undocumented
Priority:Major» Critical
Issue summary:View changes
Status:Postponed» Active

Issue summary:View changes
Status:Active» Fixed

Based on #44 this should still be postponed, not fixed?

Priority:Critical» Major
Status:Fixed» Postponed

Right, there is still the matter of whether hook_menu()'s remaining parts are going to be replaced by a different hook. Can we just leave this as postponed / revisit before release so we make sure the hooks got properly documented when they're stabilized? It's kind of important and the Docs Gate is not sufficient to make sure this happens.

Actually, I think with https://drupal.org/node/2122071 this is officially fixed. This includes not only a detailed listing of route properties at https://drupal.org/node/2092643 but also information on access-checking https://drupal.org/node/2122195 parameter up-casting https://drupal.org/node/2122223 and how to deal with dynamic routes https://drupal.org/node/2122201

It might be that there are small holes in what's here, but overall I think the "spirit" of the original post has been fulfilled.

I wonder if we want to open a less "aggro" issue, postponed and tagged "revisit before release" to do the final once-over on hook_menu() docs before shipping.

The point in #44 was that the non-routing parts of hook_menu() were being discussed to be moved to a completely different hook name -- the parts that in D7 allowed you to also make it an Administration or Navigation menu entry. Has that happened? Is it happening? I realize it's not part of the Routing system, but the original issue title here was about how hook_menu() was not documented to reflect what it actually did, so I think it's in scope before chx changed the issue title a few comments up.

Title:The router is still undocumentedDocument the internals of the router
Priority:Major» Critical
Status:Postponed» Active

I have no idea how this issue got sidetracked into relatively small changes in doxygen. Please read the issue summary. Thanks!

Because webchick asked in her AMA (even without mentioning names, I can read it well enough) I am willing to put up with the status quo if the internals gets documented and the DX gets fixed:

  1. #2109035: Make access checkers (much) easier to find
  2. #2092529: [meta] Improve DX for defining custom routes
  3. #2046367: Document the internals of the router

Here are some questions. First questions on the new handbook pages:

  1. https://drupal.org/node/2122223 First, upcast is not a word I am familiar with. I'd expect it to be unknown to others too. A little explanation would go far. I get the impression that any {entity_type} is upcast but this is not made explicitly. It does state "all content and configuration entities automatically get upcast" but it's not clear that happens by specifying {entity_type}
  2. Same page, it's entirely unclear how we get from {node} to node_load? entity_load('node') ? In other words, if {foo} doesn't work where should I look?
  3. why the beginning underscores? Are there things without underscores? If there are, what are they? How could I provide my own (a _foo like _content or _entity_view)?
  4. https://drupal.org/node/2092643 Additional options on how the route should interact. Common options are _access_mode. Again, what else is there? How to provide, how to receive?

Now, what is completely missing is the "what happens internally" part. There's a box on https://drupal.org/node/2122071 which says [Routing]. How do we route? I have a vague idea that there is some initial matching and then there is some enhancing but the what does what and how can I extend any of that is completely missing. A URL comes in and a controller object comes out, right? There's a beginning on https://drupal.org/node/2046371 but that's mostly a @todo. Again, the reason for asking this is to be able to look at something when things go pear shaped. This is why I asked for a flowchart, like an Ariadne's thread.

> Same page, it's entirely unclear how we get from {node} to node_load? entity_load('node') ? In other words, if {foo} doesn't work where should I look?

Hell yes. I was was totally baffled by this and didn't manage to figure it out.

http://slid.es/saschagrossenbacher/drupal-8-routing#/5 has a few notes about about upcasting (go down), look for the EntityResolver class for the implementation for entities.

There is a *great* post from dawehner about the underlines and routing in general on Drupal Answers.

I tried to explain a bit question one and two, see https://drupal.org/node/2122223

why the beginning underscores? Are there things without underscores? If there are, what are they? How could I provide my own (a _foo like _content or _entity_view)?

I tried to explain that, as berdir said, in my stackoverflow page.

Regarding question 4: https://drupal.org/node/2092643/revisions/view/6698717/6722443

Can we please get stuff off of stack overflow and onto Drupal.org where people will actually be looking for it? :)

It kind of answers more than the underscore question itself, but I am too lazy today to copy that into some place in the existing documentation, sorry: http://drupal.stackexchange.com/questions/89906/why-are-routing-files-fi...

@webchick: Of course, that's why I mentioned it, it should be a nice base to document that stuff better. We can't and shouldn't get it *off* DA, though.

DA is a great platform to actually find out what we need to document and what people want to know and then figure out the best/correct answer together, after which we can also put it into d.o documentation/extend it. Encouraging people to ask questions there is IMHO one of the best things we can do, various people are trying to answer any d8 question that comes up there, see @chx blogpost about that and we're doing that quite successfully IMHO. Just have a look at the answered and unanswered questions at http://drupal.stackexchange.com/questions/tagged/8 :)

Issue tags:-revisit before beta

Since this is already critical, removing 'revisit' tag.

Issue tags:+beta target

We have several somewhat overlapping places where various bits are documented:
https://drupal.org/node/2122223
https://drupal.org/developing/api/8/routing
https://drupal.org/node/2046371
http://drupal.stackexchange.com/questions/89906/why-are-routing-files-fi...
https://api.drupal.org/api/drupal/core!includes!menu.inc/group/menu/8
https://drupal.org/node/1800686

We need three kinds of documentation:

  1. Change records to document the before-and-after for hook_menu() and its ilk from D7 to D8. Already partially done in the appropriate issues, but #2244777: Document in WSCCI change notice or handbook all the menu changes (tasks, actions, contextual links, menu links) from 7 to 8 exists to cover the gaps.
  2. API documentation in the codebase for how routes are declared, especially for Drupalisms like our various route parameters. Slightly trickier because these are not something we can just stick a docblock on like we can a function, class, or class member. See #2238935: [meta] Complete missing documentation for special strings and metadata like annotation keys, routing parameters, tagged services, etc.. This should take the form of an overhaul of the routing and menu group as shown in https://api.drupal.org/api/drupal/core!includes!menu.inc/group/menu/8.
  3. A comprehensive overview in the handbook that explains not only the API itself but how to develop with it. We have a good start at this already at https://drupal.org/developing/api/8/routing and in its sub-pages, but there's a lot more to be filled in.