Tie help into menu router

Hey, have a look at the advanced help module I wrote yesterday.

LIVE FROM THE MINNESOTA SEARCH SPRINT... We're discussing how to integrate help search into search. The simplest solution is to implement a help_search, however the problem is that we don't have a registry of the available help paths. One solution is to change hook_help() to return an array, rather than have a switch statement there.

Advanced help looks promising, ... but we need to integrate help text into the search indexes, and not provide a completely new search interface that is just for help. If the advanced help module implemented a hook_search, and put this information in the search_index, then we'd be half way there, the second half being Souvent22's #252211 unified search form.

advanced help does. However, it is much more user friendly for help search to have its own UI. I actually did a lot of work to keep the search from going back to the core search pages, because it's really jarring to search help and get taken to the search paths.

"However, it is much more friendly for help search to have its own UI." Is this a comment about how bad the current UI is? I'll install your module and look at it closer.

This idea that we do "node" searches or "user" searches or "help" searches is antithetical to real use cases. As a user, I don't really think about what I'm trying to find in these terms. I just want to find something. We are talking about a unified search system, that will show results from all help on one page. So if I search for "foo", it will display the results from node, user, help, panels (something I'll probably write and submit to you) and maybe views. All of these would store a sid/type pair in the {search_index} table, and we would get the results sorted by relevancy all at once (that's the dream goal at least). If you wanted to search just help, that would be possible too.

Yea, I don't want users searching node content to get access to the help to administer Views on my site. I disagree what what you suggest is actually completely desirable, in part because access control on searching is quite hard.

i'm totally in favor of a hook_help_alter. one of the biggest hassles for me is that clients hate drupal's built in help text and there's not an easy way to change it without hacking core.

IMHO hook_help_alter() is nothing more than a way to patch up Core instead of actually improving it.

To make the help searchable we can do two things:

1. Save the help pages to the database.
2. Do a module_invoke_all('help') and let all hook_help() implementations return an array containing the help paths and contents which can be searched.

The second option allows for help pages that may change. See Dynamic Help for more information. An example of hook_help() according to the second method:

function node_help($op = 'list', $path = '') {
  static $help = aray(
    'node/add' => t('Add your own nodes here!'),
    'node/edit/%' => t('Here you may edit your node.'),
    'admin/help#node' => t('General help for node.module.'),
  );

  if ($op == 'list') {
    return $help;
  }
  elseif ($op == 'view') {
     return $help[$path];
  }
}

In this example the help is statically cached to prevent all texts from being rendered more than once during one page load. This will probably only boost performance when users are searching help (need more info on this). This is just a sketch, so please comment.

2. Do a module_invoke_all('help') and let all hook_help() implementations return an array containing the help paths and contents which can be searched.

This is *not* an option. In order to get the most gain from the registry, we need to kill the current hook_help implementation and get rid of that module_invoke_all() called virtually at every page request.

When a module is being enabled hook_help() can be invoked once to save all paths to the database. At every page request a DB call can be made to see which help paths match the current URI and to which modules the matches belong. If any matches are found, render the help by calling hook_help() only for those modules for which there are matches.

i think robertDouglass's idea of the menu system integration is probably the best one. we can reuse the existing caching and altering systems and get the help path integration for free.

1. Separate help files aren't possible this way
2. This would require rewriting the inner workings of the menu system, since normal page callbacks aren't possible for these paths anymore.

Many of these ideas are not compatible with Gurpartap's existing help system patch. As a proponent of that system, I'd like to at least see ideas that are not compatible with it integrated into it or modified or hold off until that patch is resolved.

xano, we're going to be rewriting something either way. why wouldn't normal page callback be possible? it's just hanging off of the existing callback, robert just omitted that bit.

it'd totally be possible to have separate help files. obviously we'd need to write some code but you could allow something like:

function some_menu() {
  $items['some/path'] = array(
    'title' => 'Some example',
    'description' => 'Description of the Case 1 item, with no t()!',
    'access callback' => 'some_access_callback',
    'page callback' => 'some_page_callback',
    'help callback' => 'help_get_from_file',
    'help arguments' => array('some', 'helpfile.html'),
    'help access' => user_access('some perm'),
  );
  return $items;
);

// Ideally this type of a function would be in core and load the correct file for the user's language.
function help_get_from_file($module, $filename) {
  $filepath = './' . drupal_get_path('module', $module) . "/" . $filename;

  if (is_file($filepath)) {
      return file_get_contents($filepath);
  }
  else {
    return FALSE;
  }
}

With this method you've got your help easily associated with your path. No need to for wacky preg_matches in the hook_help any longer to match certain paths.

I see what you're doing... Hmm. It doesn't look clean, since there hardly is a separate help system anymore, but it would definitely work.

1. Are the page contents of menu callbacks automatically indexed by the search module?
2. What about multiple modules adding a piece of help to the same page?
3. I don't like the idea of using separate HTML files for inline help. You'd have to extract the actual content of the <body> tag before being able to use it. I suggest using a module_name.help file for this.

As to points two and three:

function some_menu() {
  $items['some/path'] = array(
    'title' => 'Some example',
    'description' => 'Description of the Case 1 item, with no t()!',
    'access callback' => 'some_access_callback',
    'page callback' => 'some_page_callback',
    'help callback' => 'help_get', // This should be the default value
    'help arguments' => array(array('some', 'some/path')),
    'help access' => user_access('some perm'),
  );
  return $items;
);

// hook_help would be located in module_name.help
function some_help($path) {
  switch ($path) {
    case 'some/path':
      return t('Need help?');
  }
}

// Ideally this type of a function would be in core
function help_get($module, $path) {
  module_load_include('help', $module);

  return module_invoke($module, 'help', $path);
}

There's no need to bother with different languages, since all help texts are passed on through t() and should be translated just like any other string.

Xano, I don't see the need for keeping the some_help() function. If we follow the pattern used for menu titles 'help' => would be static strings and 'help callback' would used for loading strings dynamically.

I don't like the idea of one way for static strings and another for dynamic ones. I suggest using one method for both cases to keep things simple.

One more thing I thought of: how do we let help.module index all dedicated help pages? Going through all results of module_invoke_all('menu') and check whether the paths of the items start with 'admin/help' doesn't look very efficient to me.

I am still curious if pages behind menu callbacks are cached/searchable. If not, we need to add them to the search index at the same time as we're indexing them.

I don't know about actually storing help texts. This doesn't work for dynamic texts and like I said before I don't support using one solution for static texts and another for dynamic ones.

We need a concrete solution for searching and indexing dedicated help pages (I doubt we need to do this for the additional help texts at other pages). My suggestion is to let help.module invoke hook_menu() on a regular basis (because the menu isn't static) and check for dedicated help pages. It saves the paths to and titles of these help pages to its own table together with the module they belong to so it can build the help index from there. As soon as a user searches the help, help.module checks these paths, calls the callbacks (if the user has permission) and lets search.module search the content. How do we check whether a menu item points to a dedicated help page? By checking paths like admin/help/[module_name]/[foo]/[bar] for 'admin/help' at the beginning and extract the module name?

Gurpartap's help system patch already puts help text in separate, static files (Translatable) and already indexes them for a search engine. I really feel that the energy spent here would be better reviewing and extending this patch that already exists: http://drupal.org/node/299050 -- much of this conversation is re-inventing already written code.

Dynamic files are translatable as well. All you can't do with them is cache them.

No offence, but I think it's better to walk both ways and see which is best than to rely on only one.

Superior in what way? Personally I find a lot of parts of that patch are a step backwards: pop-ups, HTML that is not in theme() functions, etc. The attached files are my attempt to rewrite the help system implementing robertDouglass' idea of using the menu system. I also added a per-module permission for help pages. When I started working on it I wasn't really intending on posting it here, so don't laugh about its name :-P (help2.module). The module file is the only file that's actually working yet. Modules may define dedicated help pages by simply adding a menu callback with a path starting with 'admin/help'. Help for existing pages needs yet to be implemented by adding the 'help callback', 'help arguments' and 'help access' properties.

So far there's not really much to see, since most functions that actually _view_ help pages still rely on hook_help() being used my modules, but if you take a look at help2.module I think you get a good idea of things work.

I don't mean to go against Gurpartap Singh, I just don't like his idea and I tried to implement robertDouglass' idea instead. In the end I think it works even better than expected, that's why I'd like to continue working on it.

//edit: 'help access' is not necessary. Users that have permission to access a page should also see the help for that page if there is any.

This is anything but "needs more info". Interesting ideas in here.

#591682: Config pages are effectively hardcoded to some subcategory of 'admin/config'
and
#699848: admin/by-task is confusing since it lacks links to config pages and looks similar to admin/config and friends

both clearly identified that page-related help needs to be tied to a menu router item context instead of just paths.

#32:

page-related help needs to be tied to a menu router item context instead of just paths.

Absolutely agree, and after refocussing on this aspect, IMHO it is a task now.
Any perspective on this for D8? Help module's code is so much old-school, ugly and inflexible...

For multilingual sites now there's no ability to access {langcode}/node/xxx/edit page because of
Fatal error: Call to a member function bundle() on a non-object in /home/andypost/www/core8/core/modules/node/node.module on line 134

So filed #2092641: node_help() does not allows to edit node translations
also related #2091399: [META] Remove menu_get_object()

Coming from #788900: Deprecate and remove usages of arg() catch opened the idea to replace $path and $arg with basically the route name and route parameters.

Even I agree that this fixes some potential bugs when you replace the path but keep the route name we kind of already a similiar behavior.

I am not sure whether

* To provide a help page for a whole module with a listing on admin/help,
* your hook implementation should match a path with a special descriptor
* after a "#" sign:
* 'admin/help#modulename'
* The main module help text, displayed on the admin/help/modulename
* page and linked to from the admin/help page.

can be supported with this syntax.

At least we need to fix the following kind of bug.

Related #2183113: Update hook_help signature to use route_name instead of path - note in my (wip) patch for that I'm using help.main for the admin/help page and help.page.{module} for admin/help/#modulename

Checked with @andypost, the hook_help bug he described was cleaned up, and we have a path forward in #2183113: Update hook_help signature to use route_name instead of path, so marking this a dupe.