Convert help_main() to a Controller

Hello

Attaching patch that converts help_main() to a Controller.

Regards

The patch applied cleanly. Patch 1 didn't change the original hook_menu to use 'route_name'.

Rerolled patch should fix that.

Nice catch @frob, I forgot to include help.module changes in my previous patch.

Re-rolling patch indenting doc and removing unused __construct() method in HelpController class.

Regards

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,70 @@
+    foreach (module_implements('help') as $module) {
+      if (module_invoke($module, 'help', "admin/help#$module", $empty_arg)) {

Both module_implements and module_invoke could be replaced by the module handler: 'module_handler' which is in the container.

Yeah! go for it!

Re-rolling patch using the module handler.

Thank you.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,83 @@
+  public function __construct(ModuleHandler $module_handler) {

This should use the interface and could get some documentation on the object.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,83 @@
+   * @return

Should be @return string

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,83 @@
+  }

You should have an empty line before the last } of the class.

Re-rolling patch according to last advices.

@dawehner Thanks for your efforts.

Beside from these two small things, this is ready to fly!

+++ b/core/modules/help/help.moduleundefined
@@ -12,8 +12,7 @@ function help_menu() {
     'file' => 'help.admin.inc',

No need for file anymore. sorry I should have seen this before.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,90 @@
+   * @var \Drupal\Core\Extension\ModuleHandler
...
+   * @param \Drupal\Core\Extension\ModuleHandler $module_handler
...
+  public function __construct(ModuleHandler $module_handler) {
+    $this->moduleHandler = $module_handler;

:( Should be ModuleHandlerInterface

No worries, here is the definitive patch, I hope :)

Regards.

Oh, is there a reason why you don't remove the previous code?

Oh sorry, it was a regression generating patches.

Re-rolling...

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,90 @@
+  private function helpLinksAsList() {

Urgs. Drupal just uses proctected

Re-rolling changing function definition.

+++ b/core/modules/help/lib/Drupal/help/Controller/HelpController.phpundefined
@@ -0,0 +1,90 @@
+    $output = '<div class="clearfix"><div class="help-items"><ul>';
+    $i = 0;
+    foreach ($modules as $module => $name) {
+      $output .= '<li>' . l($name, 'admin/help/' . $module) . '</li>';
+      if (($i + 1) % $break == 0 && ($i + 1) != $count) {
+        $output .= '</ul></div><div class="help-items' . ($i + 1 == $break * 3 ? ' help-items-last' : '') . '"><ul>';
+      }
+      $i++;
+    }
+    $output .= '</ul></div></div>';

We should certainly open a follow up to convert this into a themable function.

Yeah, a follow up issue for clean the way the output is generated could be a good option.

Then, we could mark as RTBC this issue if you consider it and open a follow up taking as base this code once it will be committed.

What do you think?

Regards.

Sounds like a great plan.

Committed 51dc76b and pushed to 8.x. Thanks!

Follow-up issue created: #1986164: Improve the way main help page is rendered

Regards.