Add a config entity for a configurable, topic-based help system

Image references -- no, but you could certainly have an image that is somewhere in a module directory or subdirectory, and make a direct IMG tag to it.

SystemHelpBlock to Help module -- nothing to do with this issue. Please file a separate issue if you want to do that. SystemHelpBlock is for the help that goes at the tops of individual pages. This issue is about the module-level help topics that appear on their home page. Totally separate.

Tours are for pages. System help blocks are for pages. This config entity is not, and I don't think it serves the same purpose as a tour. I also don't think the Tour entity has the same structure as a Topic entity, and I don't think I want to make an entity complex enough to know what page it belongs on, or take over hook_help() completely. Please... don't try to make this any more complex than it is!

Also this issue has been marked Beta Deadline, and we are past beta. We do not have time to make it Do Everything Possible That You Could Think Of.

OK, I am updating the issue summary to describe the limitations of this idea.... Actually it already says this. Emphasizing it.

THIS IS ONLY for the help topics that you see on admin/help. It is not for the help at the top of pages that the System Help block shows.

I have a preliminary version of this working! This is NOT DONE but I am going to upload this patch just in case my computer crashes or something... still some work to do! I'll be working on it more tomorrow.

Here's a more viable patch. I didn't make an interdiff; quite a lot has changed, and hopefully no one looked at the previous patch yet since it was just up there to prevent me losing everything in a computer crash.

There are a few things that still need work in this patch, but it's basically working and I would love it if people would try it out. The things that need work that I know about:

a) Breadcrumbs on topic pages -- need to make a breadcrumb handler class so the breadcrumbs are Admin> Help like on Module pages. Fairly easy to do; a couple of examples in Core (Book, Comment modules).

b) Internal links. I think this just needs to be done via a text filter, which would almost undoubtedly be useful in general for Drupal admins and content editors. I've tentatively made a Help module topic config. In this, I used things like <a href="[[help.main]]"... which I would propose this text filter would transform to a link to admin/help (which is at route help.main). I'll work on this next.

c) Tests. There aren't any, and this could break existing tests? not sure.

d) A few more topics should be converted. Once I have the text filter working, I'll work on this too.

Suggestions/notes if you want to test it (turn on the Help module obviously):

- Go to admin/config. You'll see an entry for "Help topics" that will take you to the config page, where you can add/edit topics.

- Go to admin/help. You'll see the previously-existing list of module help topics, and a new area listing the configured help topics. We can bikeshed about the wording/headings starting now. :)

- For the moment this patch only has one configured help topic, for "Help Module". This explains how to use the configured help stuff.

Thoughts?

I'll change it to admin/help-topic/* in the next patch. I don't think help-topics (with an S) is a good idea -- we don't have nodes/5, we have node/5. And taxonomy/term/3 not terms. Etc.

Your idea for the naming convention is fine. I'll get that into the Help class doc. I don't think I want to mess with complicated URLs like that though. admin/help-topic/ID should be it. Period. Too complicated ==> difficult.

Oh wait though. We can have a module and a theme with the same machine name... so I think just using the machine name of the extension without a suffix is not a great idea. I suggest we adopt modulename_module and themename_theme as the convention.

OK, here's the next bit: I've fixed the breadcrumbs, changed the route path as discussed above, and fixed the tests; also added tests for the new functionality. The interdiff may or may not be OK, it's a bit weird due to adding files.

As a note, the existing HelpTest was assuming that modules implementing hook_help() would have module overview pages. This is increasingly not going to be true with this patch. So I just tested with a new test module instead.

Remaining item to do is to get the links working, and that part will need tests.

Yes. I plan to put another few into this patch, and then have a meta afterwards to convert the rest. Glad you like it! :)

Here is a new patch that fixes a few problems with Tim's patch (thanks Tim!) and also implements the change he suggested of making the entity class HelpTopic, with ID help_topic and config prefix topic. And also the suggestion of making the convention for module Foo help to just have machine name foo not foo_module (if Tim and Vijay agree this is better, I will bow to their wisdom).

The interdiff shows changes in the files, after renaming two files:
Help.php -> HelpTopic.php
help.help.help_module.yml -> help.topic.help.yml

Still need to do something about the links. larowlan is working on a token solution (brilliant!).

Great! With larowlan's tokens, here is a new patch. Added a token::replace call to the Controller and changed the help topic config so it has [route:] in it.

The only other thing I think we'll need right now for the 90% use case is a token like
[help_topic:machine_name]
that would turn into a link to the help topic. Or something like that. The current patch, which just converts the Help module topic, does not need this.

Oh, one other thing I thought of.

There's a @todo in the Controller that says something about "maybe list the other topics that list this one as Related".

I am thinking now that we shouldn't do that. But what we should do is have another field on the config entity for "show_on" that would say "Also list this topic as Related on these other topic pages".

In this way, for instance, contrib module A could say "List me on this other module's topic" and it would only show up if that other module was turned on, and the other module wouldn't have to know about module A.

So I'll add that tomorrow, and if larowlan doesn't beat me to it, also do something about topic-to-topic link tokens, because we need those (the route tokens do not support placeholders, because how could you do that?).

uh. Something weird happened with the files there... :(((((( ok hopefully they are all there...

This is most excellent! I go away for about 18 hours and there are new patches and new reviews!

I'm going to make a patch now to address hopefully the remaining concerns... thanks vijay especially for summarizing the status on #28 to that point!

Just adding this comment now; if someone else is working on the next patch please ping me in IRC now so we don't cross-post! Thanks everyone!!!!

Here's another patch, which addresses some mostly minor things from the previous reviews. I will make another patch or patches for some of the larger issues, on a separate comment or comments.

Note: two interdiffs -- one for the changes to existing files, and one for the new files (see below).

Replies and notes about the above reviews:

Comment #30

#8 - Anything that can provide configuration can provide topics. This would be: modules, themes, profiles, and Core. So... "modules, themes, etc." is probably OK.

#14 - We are maintaining the possibility that module developers (only modules!) can also use hook_help() to provide module overview topics (there are actually reasons you might want to use a function, because for instance it can be dynamic, and for backwards compatibility at this point in the Beta cycle). Because the Core modules are expected to mostly or entirely be converted from using hook_help() to using config entities, the test module uses hook_help() for testing purpose. So that needs to stay.

But I did also add two topic entity configs to the test module, and some additional tests.

Comment #33

The fix in #28 for this was not really complete: if you edited a Help topic and then export it, you would be back to having it all in one line with \r\n in there. So this still needs work.

Comment #39

Addressed all in this new patch.

Comment #41

#1 - This predated this issue, but good idea. Fixed.

#2 - Ah, great example! Agreed this is a good idea. TO DO

#3 - fixed

#4 - When I took out that line, I got and exception when submitting the form:
Notice: Undefined index: exists in Drupal\Core\Render\Element\MachineName::validateMachineName() (line 220 of core/lib/Drupal/Core/Render/Element/MachineName.php).
So I think it is necessary.

#5 - Excellent, duh! Fixed.

#6 - I can't convert topics from hook_help() to config entities without the added token types, so I'd like to keep this here (I think this comment is saying "do the routes token stuff in a separate patch", right?). Topics need to have links to routes and to other topics.

#7 - reverted this change, agreed not related to this patch.

#8 - indentation fixed, I guess. It's a two-level-deep array.

Other To Dos

- Create a test for the admin pages.
- Add the ability for help topics to say "List me on this other topic page".
- Convert a few more Core topics

Added notes pertaining to batigolix's comments in #40 to the issue summary.

I'm still working on the remaining To Dos listed in #43.

RE #45, the main reason for the trim is that in the next line, where it only adds items to the array if they are non-empty, I also wanted to weed out empty items that are only whitespace. I suppose it's not really necessary but seems cleaner.

Anyway. Latest patch and notes:

a) Convert to using a proper entity view builder instead of the controller method [comment #41 - item #2].

b) Add ability for topic A to say "List me on topic B page", and adds tests for this.

c) I did some further investigation on the \r\n problem. It seems that those \r\n are actually necessary... So if I go into the Add Help or Edit Help form, with CKEditor turned off, and I format the HTML source so that I can actually read it (i.e., put in newlines and maybe indents), when I go to config export it puts it all into one line and with the \r\n in there. And if I import a version of the help module topic page config without the \r\n in it, when I go into the Edit page, it's all scrunched up together into one line and hardly readable by an editor.

So while the config now accepts multiline, it's not really all that great if the source is HTML and you actually want the newlines kept in there where you put them.

d) I realized the "Full HTML" text format is probably not a good idea for shipped help topics, because that's part of standard profile. So I made a text format for help and added that to the help module.

e) Added a local task group so that View and Edit are local tasks (tabs) for help topic pages.

The only other To Dos, as far as I know, are:
1. Convert more topics. vijaycs85 is apparently working on this.
2. Add some tests for the admin CRUD functionality. I'll do this next but wanted to get this patch up for vijaycs85.

Two interdiffs again... one for existing file changes, one for new files.

I added a child issue for the additional conversions.

And here is a patch with the admin CRUD test. Again, two interdiffs, one for two minor changes uncovered by the test and while writing it; the other for the new file.

Phew. I think this might be ready to go, of course pending the reviews!

forgot one

Regarding #52 -

The text format defaults to using the "Help" format, actually, which is supplied by the Help module in this patch (if you had previously had the Help module installed and you just updated this patch, you will need to uninstall/reinstall Help module to get the new config imported for the text format, and you'll see this default). I do not think we should default to Basic or Full HTML, as those are part of Standard install profile, so they are not necessarily present on any given site. We also should not save any Core module help topics with anything but the Help text format, because that is the only one guaranteed to be present. So... I think defaulting to the Help format is actually the right thing to do. Thoughts?

Agreed the Related fields would be better as something more like either an entity reference or like tags... It would be easy to change the UI later, though, and I think it's good enough as it is now, and the storage is what we want. So... I filed a follow-up so we can discuss a nicer UI for this. OK? #2354955: Improve the UI for filling in help topic references

Regarding #51 & #52 - good cleanups, fixed in new patch.

I guess we are going to need a change notice for this, although hook_help() still is quite usable in its old form and isn't changing: https://www.drupal.org/node/2354963 is the draft.

@larowlan: Are you saying we should get a usability review on the current UI, or for the proposed new UI? The UI that is there is just "enter a comma-separated list of topic machine names in the box".

Filed one more follow-up. #2355685: Make "view help topics" its own permission Not sure if it's a good idea or not but thought it should be discussed.

RE #63 - There is a translation tab if you install this module.

RE #62 - OK, I'll look into this... or if someone else has time to figure it out, go for it. I have to catch up on some client work this week and I'm heading to the PNW Drupal Summit in Portland for the later part of the week. So I'm not sure when/if I'll have time. :(

Added one more follow-up issue, which also depends on a different issue being fixed and so can't be done right now (but it will be about a 5-line patch I think): #2356015: Convert Body field on Help entity to use new text-with-format

I looked at how the Tour module does this... As far as I can tell:
a) Tour config schema has a "module" element.
b) Tour entity class has a corresponding $module member variable.
c) Tour entity class in calculateDependencies() does

$this->addDependency('module', $this->module)

I guess we could do this, but ... Tours do not have an editing UI, and the help entities do. What are we supposed to do about the module element in the edit/add topic form?

Alternatively, is there a way, when the config system is importing a Topic from a config/install directory, to have the config system set the module to the module that provided the Topic automatically, and then I guess just make it so that anything you create from scratch in the UI sets the module to 'help'? Then we'd have to tell module developers who create help entities using the UI and then exporting, would need to edit the file and change the 'module' element to their module name?

And... what about topics provided (hypothetically) by themes?

I really don't know about this... Any thoughts? And really, the config system doesn't automatically remember where config came from and uninstall it when the module that provides it is uninstalled? It seems like since config can be provided by modules, themes, and install profiles, as well as Core, that asking the config entity to figure out where it came from and deal with this is too much to ask... really do we need to do this?

OK, this is great, thanks everyone for looking/testing/reviewing!

Things that need to be done:

a) Make sure the help test module is hidden. Oops. Thought I had done that.

b) Make sure help topic entities are displayed in the right language. The problem is that entities as route parameters are not translated, because it's assumed you are editing them (and that needs to be in English).

Berdir's suggestion for how to fix this is:

1. Add some information to the topic display route:

entity.help_topic.canonical:
  path: '/admin/help-topic/{help_topic}'
  defaults:
    _entity_view: 'help_topic.full'
    _title: 'Help'
  requirements:
    _entity_access: 'help_topic.view'
    options:
      parameters:
        help_topic:
          type: entity:help_topic
          # Force that help topics are loaded in the current
          # interface language
          use_current_language: true

2. Modify AdminPathConfigEntityConverter::applies() so that if it detects that use_current_language: true, it will return FALSE and so the default entity loader will be used and not the "if it's admin load in current language" loader.

c) Figure out dependencies. I think we actually should allow each topic to have multiple module and/or theme dependencies. So the schema would have something like:

dependencies:
   type: sequence

and within that each dependency would look like "module: foo" or "theme: bar".

The question is whether we should allow this to be specified in the UI. I think since these are editable by admin users, we can just (for now) make it a big text box comma separated. So if a help topic writer wanted to say "This topic depends on modules foo and bar, they would enter:

module: foo, module: bar

in the box. That would at least be clear and editable and would work. We could refine the UI later if necessary.

Thoughts?

doh! berdir to the rescue -- in (a) I apparently put the test module in directory "test" not "tests". That will fix it!

in (c) I will need to call this something other than dependencies in the config schema, because dependencies are dynamically, automatically calculated.

OK, one more try on (c)...

On #2224581: Delete forum data on uninstall the patch introduces fixed config dependencies. That isn't done yet. That is what we would need.

So what we would need to do is have this editing field where you could enter module: foo, module: bar, theme: baz, but when saving, put it into

'dependencies' => array(
      'fixed' => array(
        'module' => array('foo', 'bar'),
        'theme' => array('baz'),
)

in the entity.

I'm not sure what to do in the meantime...

To start with, here's a patch that moves the core/module/help/test directory to be core/module/help/tests -- no other changes from the patch in #53. Should take care of the "test module is not hidden" problem. I didn't make an interdiff file.

Good. So my plan for the "translation not working" issue is:

a) Write a test that installs help_test, help, and config_translation or whatever they're called. Adds a language. Clicks on Translate for one of the help_test topics, and supplies a "translation". Views the topic in the other language and verifies that it's displayed correctly. This would verify that the translation screen is there in the UI as well as that the topic is displayed correctly.

b) To fix it so it actually works: do as outlined in #71 item 1. Sounds like we all think this is a good idea, and it's also fairly easy to do.

So that is the next piece to work on.

I'm still not sure about the dependencies problem, but if I'm lucky, while I'm working on this that forum issue will land and we can use that method. :)

Yeah, but I think we want to allow for both modules and themes to have topics, so rather than a "module" field, we'd want to have a more generic dependencies field -- like the fixed dependencies added in that Forum issue.

Updating the issue summary with the two remaining To Dos. I'm going to work on the plan in #77/#71 for fixing the translation problems.

Here's a fix for the translation piece. It works for me, and has a test.

Question: There was already a Translate operation link on the Help topics management page (and this tests verifies its existence). But how can I make it so that if you are on the Help Topic edit or view page, you get a Translate tab (local task)? Shouldn't the config translation module put that in there? It isn't... Anyway, you can get to the Translate page from the help topics admin page, so maybe this is OK.

Anyway, two interdiff files (one for the changes and one for the new test file), and a new patch.

Ugh ugh ugh.

So I tried to do the simple thing:

function help_config_translation_info_alter(&$info) {
  $info['help_topic']['base_route_name'] = 'entity.help_topic.canonical';
}

This doesn't work at all, because there are other assumptions all over the config translation module that assume that the base route is really the Edit form. So for instance, with this change you cannot go to the help topics admin page at all because of this code in config_translation_entity_info_alter():

      elseif ($entity_type->hasLinkTemplate('edit-form')) {
        $entity_type->setLinkTemplate('drupal:config-translation-overview', 'config_translation.item.overview.' . $entity_type->getLinkTemplate('edit-form'));
      }

which is just blindly assuming that the base route is the edit form, which if you've altered it, is not true. And even if you comment out a bunch of code to get past that, and go to the Translate overview page, the Edit links on the Translation overview page always go back to whatever the base route is, so you have an Edit button that takes you to the View page if you've made this alter...

So... The assumption that Edit is the base route for translating config is deeply built into this module and is WAY beyond the scope of this issue to fix, IMO.

:((((((((((((((((((((((((

I realy don't know what to do. This isn't going to work without a lot of tearing apart the config translation module and making it actually obey and use its own hooks and not having assumptions.

OK. For the moment, the way to get around this is to make Edit be the default tab and the base path. This patch does that. The UI is now OK -- you get edit, view, and translate tabs in the local task group. View is 2nd, which is slightly odd, but not horrible I think? The path admin/development/help/ID now goes to the edit page (used to be .../ID/edit), and admin/development/help/ID/translate is the translate path.

I think this works... tests pass locally... tabs are there... links work.... Thoughts?

Also I filed a separate issue about the inability to use the canonical route as the base route for translation:
#2358923: Config translation module cannot deal with different base path

Not sure what you're asking in #88?

I thought of one more thing that this patch should have: validation for the two "related" fields in the Form classes. They should only contain commas and machine names of topics. I don't think we should validate that the machine names exist, but I think we should validate with a regexp that the fields only contain commas and whatever characters are allowed in standard machine names (we're just using the standard machine name regexp for the machine names).

I'll take care of this later today (after I actually wake up) or tomorrow...

OK, here's a new patch with validation.

Right, help.page is the route for static topics defined through hook_help(), and entity.help_topic.canonical is the route for editable topics defined through these new config entities.

The "workaround" for config translation in this patch is only that I made Edit be the first tab in the local task group instead of having View be the first tab. This is backwards from what you see on Node, Taxonomy term, etc., but at least all the tabs are there and work, and the URLs are what we will eventually want them to be. We can easily adjust this if the underlying bug is fixed in config translation, but right now I do not think it is possible to make this work with the tab order being View, Edit, Translate... as outlined on #2358923: Config translation module cannot deal with different base path.

So the only other remaining thing to take care of is the config dependencies. We're waiting on #2224581: Delete forum data on uninstall; it is currently RTBC so... maybe soon?

#2224581: Delete forum data on uninstall got in, so here is a new patch with dependencies, with tests for the new dependencies stuff, and a UI for adding them in the editor for help topics.

This should be the final iteration, pending reviews. Yippee! Tests pass locally...

Summary update.

RE #97 -

item 1 - seriously? We have no standard about putting @var on every single local variable. In this case it's the return value of hook_token_info(), and we certainly have a standard saying that "Implements hook_whatever()" is acceptable documentation in this case. This is a 10-line function that returns an info value for this hook, and this local variable is clearly an associative array... nothing much to document? If you think it needs docs, please make your suggestion in the form of a patch? I don't think it's necessary.

item 2 - This is a separate issue and out of scope for this issue: #2337317: Replace help page layout CSS with reuseable layout classes

item 3 - Doesn't sound like you want a change.

item 4 - This is used as the "exists" callback in the machine name form element:

+    $form['id'] = array(
+      '#type' => 'machine_name',
+      '#default_value' => $this->entity->id(),
+      '#machine_name' => array(
+        'exists' => array($this, 'exists'),
+        'error' => $this->t('The machine-readable name must be unique, and can only contain lowercase letters, numbers, and underscores.'),
+      ),
+    );

Regarding #99, we have code like this all over core... Add it if you want. :)

Regarding #100... The main (only?) use case for this field is "Contrib module or theme developer making help topic(s) for distribution with their module/theme". So, the users of this field are likely developers, and I don't really think multi-selects are necessary, or careful validation. It's really there just for the convenience of developers (of the core help topics we need to convert now, and contrib down the line).

Given that, what if we just change the Description so it says something like "For use when creating topics for distribution with a module or theme", and leave it at that? [see attached patch]

And by the way, there is some minimal validation (allowed characters), just not the full validation of making sure the listed modules/themes actually exist.

So... I'm out of town the rest of today through next Monday. If something else comes up and someone wants to make a new patch, please do!

Was this approach OK or not?

We need to evaluate whether this should be:

a) Added to Core now.

b) Postponed to 8.1.x or a later release

c) If (b) published as a contrib module now

I am adding some information to the issue summary about the technical debt and options for this.

RE #104 - yes, it should save time in the long run and make it possible for actual writers who are not coders to write module help. Nonetheless, I was asked to list the technical debt that would be incurred if this patch were accepted post-beta, and therefore listed the need to convert existing help as one aspect of that. The 8.x branch maintainers have to evaluate technical debt/risks vs. rewards when deciding whether this belongs in 8.0.0, a later release, or contrib.

Meanwhile... any further issues here? I am not sure that the decision will be made until this patch reaches RTBC (again).

The patch in #101 has been at Needs Review for 10 days... It would be really nice if someone could either mark it as Needs Work, RTBC, or "This needs to be postponed to 8.1.x". Thanks!

RE #109...

First comment, OK, sorry, wasn't aware of that standard.

Second comment: Using the standard profile in this test was introduced by this issue's patch. All this patch does is add a better comment explaining why exactly it is being used, and move the line that was already there up a few lines.... So yes, I agree that it should not really be using standard profile, but filing this as a separate issue rather than killing even more kittens by trying to fix every problem in the Help module in this one patch: #2368277: HelpTest should not use standard profile (postponed until this one is resolved one way or another; this issue is marked as "related" on that other one so we don't need to add it as "related" here).

So here's an interdiff to take care of the blank line issues for all the classes in Help module. Setting back to RTBC since this is a trivial all-whitespace change.

Well if we classify this as a feature request then we must postpone it to 8.1.x or later. Which is disappointing, since I really do think this is a good system and also fills a need, but fair... it just simply came too late.

So, I'm not going to bother working on a new patch right now. Good comments in #112 though, thanks!

If we did want to have this as a contrib module in the meantime, there are some parts we would need to break out of this patch since they are not in the Help entity itself, but a contrib module would not work without these changes:

a) core/lib/Drupal/Core/ParamConverter/AdminPathConfigEntityConverter.php -- a change to allow admin routes to have use_current_language parameter that makes the config entity get translated on output

b) core/modules/system/src/Tests/Token/RouteTokensTest.php and core/modules/system/system.tokens.inc -- token "route" that outputs the correct URL for a route.

I have no idea whether either of these changes would be considered to be less feature-like... (b) could be added in the contrib module but not (a), and that would block any possibility of this working as a contrib module.

Any thoughts on that?

But... I'm not really all that inclined to go the contrib module route on this one. I don't think it's worth making another contrib module alternate help system. If it is not in Core then contrib modules would need to do their help twice to support both the existing core hook_help() and this new system, and that's extra work they're unlikely to want to do. There is no guarantee we'd even get this into 8.1.x either, and who knows when we'll actually get 8.0.x out the door, much less 8.1.x.

So... Let's just leave this as 8.1.x. and see what happens a few months/years down the road. Thanks for all the help from everyone who worked on this! Sorry it didn't work out yet. :((((((((

I also just want to say that I really really really really wish that when I originally discussed this in IRC with alexpott in early October, or in an early comment, I would have known then that it was too late rather than spending a month trying to get this done. I feel like I was told then that it might be possible when it was really not ever going to be, and that led to me wasting a very large amount of time that I could have spent fixing issues that could have gone somewhere. I don't entirely disagree with the decision but I wish it had been made much sooner instead of being made today.

No, it wouldn't break anything. But this issue has been classified by the core committers as a feature request, and that decision is reasonable. Therefore it is not allowed to proceed at this point. The policy is here: #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?.

So... Unless it is no longer classified as a feature request, that is the end of the discussion, because the policy is very clear that feature requests are not allowed at this point. My only annoyance is that it would have been nice to know that this would happen a month ago when I discussed this in IRC and specifically asked if it was too late. But these things happen.

@alexpott: You are completely right. This is a feature, and it's too late. No worries! We all should have thought of that a month ago, and we didn't. Oh well, these things happen.

@catch: Good question in #119. I will add that to the issue summary, since even if this becomes a contrib module the aspect of modules updating their config will need to be addressed.

Anyway... I've just updated the issue summary with a list of the outstanding concerns and the present status. I'll think seriously again about making this into a contrib module in the next few weeks... It would sure be helpful for site builders and certain contrib modules, and the Docs Working Group may also be thinking about using this to build a translatable basic User Manual for Drupal 8, which we've been needing forever.

Yes, the contrib module could provide this token. It's not really related directly to the help topic entities but as I tried to say in #113 (b), there is no technical reason it cannot go there. I forgot to add that to the issue summary, so doing that now.

I also filed a separate issue for #113 (a), which I forgot about again (thanks for the reminder): #2369035: Config entities should not always be untranslated in admin routes

And I don't know how to resolve the config changes issue either. It's already in the issue summary... but yeah, good question.

@andypost - great! A co-maintainer would be lovely.

Making a sandbox will take some work, because the current patch is for the core Help module and other Core files. But it won't take too long... I don't have time today but will set it up later this week if possible. I don't think it really needs to be split up into the tokens and entity separately, though -- the token part is small and the entity has a token too (for links to the help entities), so ... I think it's best just to put both into the same module as the entity. You really will need those tokens for most help topics.

So right now the main blocker to even having a viable contrib module is #2369035: Config entities should not always be untranslated in admin routes. If someone wants to make a viable patch there (take the bits out of the patch here), that would be great! It will most likely need a test though, which will take some work... this help entity made a good test of that behavior but... probably something simpler can be devised? I hope.

Assigning this back to me, as next steps are mine.

I can think of a couple of approaches to the problem of "Module updates its help topic":

a) We could define a standard functions they could call from a hook_update_N() like help_topic_updated($id) and help_topic_added($id). These functions would somehow check to see if the user had ever edited the help topic for updated (possibly we'd add something to the config entity to detect this?), and see if a topic with that ID exists for additions, and then do something like this:
- Updates - If no edits had taken place, update by reading the new config from (module)/config/install
- Updates - If edits had taken place, tell the user a new version is available and let them act.
- Updates - if the topic doesn't exist, the user must have deleted it and leave it alone
- Additions - if there is a topic with the same ID, warn the user and let them act.

That would require a module maintainer to define a hook_update_N() and call these functions, but that probably isn't too much of a burden. We'd also have to add some stuff to the UI like "Show differences between this currently configured help topic and the shipped version" and "Reset this topic to the version that came shipped". And we'd have to have one or more flags on help topics to indicate whether they were shipped config and/or had been edited since then.

b) The help topic module could have a hook_modules_updated() or whatever it's called, which would compare the (module)/config/install topics to ones that are currently in the config system and warn the user if there are differences (updates, deletions, additions), whenever updates were being performed.

c) We could also implement hook_modules_updated() and just generically put out a message that warns the user when any updates are done that help topics are not updated or imported and let them figure it out with this hypothetical UI.

d) We could do nothing, on the assumption that help topics are site configuration and you don't need the updates.

I'm not sure which of these is the most feasible or which is best... or is there another idea that would be better? Or do you think it was stupid to consider making help topics be configuration at all? I think having them editable is a really good feature; on the other hand, it does definitely complicate the "acting as a manual for an extension" feature. Probably not in an insurmountable way though.

I've made a preliminary commit to making this a contrib module in a new sandbox project at:
https://www.drupal.org/sandbox/jhodgdon/2369943

I haven't tested it yet... it will not work right without the patch for #2369035: Config entities should not always be untranslated in admin routes at a minimum. It is also quite likely that I messed some things up when I tried to make everything have the module shortname "config_help" instead of "help", so I expect it won't work yet. I'll work on it more tomorrow.

ALSO you need to uninstall the core Help module before installing the sandbox/contrib module! It defines some routes for the same paths. Anyway... don't try it yet I'm sure it's broken!

Tomorrow I'll also make the To Dos into issues on the new sandbox project and also probably move some of the child issues over there.

OK. I got the sandbox module working! At least the tests pass. See #2371443: [Meta] To Do list for Configurable Help for the To Do list, or just look at the issue list for this module:
https://www.drupal.org/project/issues/2369943
I moved several of the "related" issues over to that project so we can work on them there.

The module is at https://www.drupal.org/sandbox/jhodgdon/2369943

You'll need the Core patch to #2369035: Config entities should not always be untranslated in admin routes installed, and at least right now, this is a drop-in replacement for the core Help module so you cannot run the core Help module at the same time.

Update on the contrib module for this:

I've been working on this module for a while. It's now a separate module that depends on Core Help instead of a drop-in replacement, and I've fixed most of the issues that were identified here, as well as a few more that I came up with. I'm pretty happy with it now, and will probably turn it into a Real Project sometime soon.

The only missing piece that I know of is around what to do when a module updates a topic -- now issue #2371439: Figure out what to do when module updates configurable topic -- which was brought up by @catch above around comment #119 and discussed a bit in later comments. I think I've identified an approach for resolving this issue, and I'll be working on it. Incidentally the core Tour module needs this same functionality; see #1924202: Tour tips are provided as configuration, so never get updated... at least, I think that is what that issue is about, and if not they probably need a new issue for this problem.

Anyway... If anyone wants to try out the module, it's now named "Configurable Help", it's at https://www.drupal.org/sandbox/jhodgdon/2369943 , and your module directory should be called "config_help". It seems to be working well and all of its tests pass. The issue queue for the module is currently a nice sea of green aside from two postponed issues and the "what to do about updates" issue mentioned here. I'd love it if people tested the module and filed more issues. Well. I'd at least feel more confident. :)

Thanks!

I've filed a new meta issue to collect all the problems with the current help system and discuss the best route to fixing them; this issue is being added as Related there:
#2592487: [meta] Help system overhaul

I'm thinking about reviving this for 8.1.x, and I guess now would be the time...

I think now that we should leave the About sections of the existing hook_help() as they are, and make the Uses sections into a configurable topic outline that would be organized by topic (how to do things) rather than by module as it is now.

So if you go to the Help page, you'd have 3 sections: the module overview pages from hook_help(), the topics outline (which users could add more pages to, rearrange, edit, etc.), and a list of Tours as well.

Speaking of which, I don't think we ended up ever figuring out what we needed Tours for in core and making those tours... we should do that too.

I've spun off part of this to #2661200: Make admin/help page more flexible, and list tours on it and will work on that first.

Updating the issue summary and title, as my current plan is not to get rid of the module-level help in hook_help(), after discussion this with @alexpott a few days ago. I also discussed this with @webchick in IRC today... Anyway, I'm planning to go forward with making a patch for this sometime soon. However, it will depend on #2661200: Make admin/help page more flexible, and list tours on it getting done (which allows for modules to place sections on the admin/help page). And I think I will make the configurable help be a separate module.

I also cleaned out a bunch of obsolete stuff that was already fixed from the issue summary.

So while I work on getting the patch for #2661200: Make admin/help page more flexible, and list tours on it refined, I'll be working on the configurable help system, and some topics for it, in the sandbox project https://www.drupal.org/sandbox/jhodgdon/2369943 and its issue queue. I should have a patch here shortly after that one converges towards RTBC.

OK, I'll get back on this!

Right now my sandbox module mostly needs (a) some unit tests and (b) some topics to ship with... If anyone wants to help, the issue queue for the sandbox is:
https://www.drupal.org/project/issues/2369943?categories=All

I haven't tested it lately against 8.2.x, but that would be another good step. It should be OK, as it was assuming the patch for that other issue got in, in the latest iteration. There are existing tests so if they all pass then that would be a good sign that the module is working.

I know what you're saying, but as someone who used to build sites for clients, I also know that it would have been very helpful to be able to pick and choose topics from those provided by Core, and add my own, to build a help system for my clients. At least being able to edit the "outline" would be necessary for that, and ideally, you'd want to also be able to edit the core topics (for instance, you might be using a contrib module that modifies some workflow).

One thing that we could do is to have a field in these config objects that indicates "This was provided by a module/theme". If this flag was set, we could do one or more of the following:
- Require a more restrictive permission to edit the topic [I think this is a bad idea]
- Display a message about the dangers of editing the topic [I think this is a reasonable idea].

To me, this is not much different from allowing people to delete/edit the default views that come with a module, even though if they screw them up somehow or delete a display that the module is expecting will exist, possibly the admin or user-facing functionality of the module may not work.

Anyway, I've filed an issue in my sandbox to discuss this:
#2687787: Provide a method for locking topics against editing

Well, I think my sandbox is in pretty good shape, so I redid it as a Core patch. It's kind of large... and it's quite different from the patch in #110, so I didn't think an interdiff would be helpful...

Short roadmap:

a) There's a new plugin type called TextSection, which I put into core/lib/Drupal/Core/Render (also it has an annotation class, and a plugin manager/service, and a render element in the TextSections class). This allows you to take a large piece of HTML-formatted text, and abstract it slightly to "sections", where a section could be a paragraph, header, sub-header, bullet list item, etc.

b) There's a new module called config_help, which defines a config_help config entity for help topics, provides a UI for editing them, and lists them in a new section on the admin/help page.

c) To avoid having very large pieces of HTML-formatted text in the body of the help topic config entities, which is bad for translation, they're made up of TextSection items instead. However, the config entity and its schema don't really need to know about TextSection plugins -- the help topic YML body looks something like this:

body:
  -
    type: paragraph
    text:
      value: 'Follow these steps to build a help system for different types/roles of users on your system to use:'
      format: help_plain
  -
    type: numbered
    text:
      value: 'Plan your help system: make a list of the topics each type/role of user would benefit from.'
      format: help_plain

(etc)

d) To start with, I only defined help topics for the config_help module itself, although I've started writing some additional topics in my sandbox project. I think that the initial patch should just do this much for topics, and then we can do more topics on a child issue.

e) There are tests! One Kernel test for the TextSection stuff, and 4 web tests for the help topic stuff.

Looks like I need a Composer entry for the new module, and the other test failure seems to be a bot problem.

Anyway... I'm actually thinking I should split the new render stuff off into a child issue and get that done separately first, and then once again come back here with the rest. I think that will make it all more understandable and reviewable. So... here is:
#2697791: Add Plugin system for abstracted HTML-formatted text
and I'll split that part of the patch off there, and postpone this one again...

I'm postponing this issue again. On #2697791: Add Plugin system for abstracted HTML-formatted text, a suggestion was made that instead of the plugin system for the body of the help topics, it might be better to use a markdown system instead. This is being discussed on #2698035: Add markdown system for HTML-formatted text. Until one or the other is decided on, this issue cannot proceed.

Well, the idea here is for modules, themes, and install profiles to provide help, too. I see what you are saying about it being content -- and it's been brought up before. But if extensions are providing it, making it configuration means that we don't have to:
- Think up a new way to provide things in extensions (config entities already do that)
- Think up a new way for the POTX packages to find translatable strings (config entities are already picked up by POTX so they are translatable on localize.drupal.org)
- Think up a new way to combine the things that are provided by extensions and things that are provided by users (config entities already do that)

This is why I built it as config entities.

Regarding diffs, see this contrib module:
https://www.drupal.org/project/config_update
This is also a problem with views that are provided by core modules vs. views created by users.

After more than two years of trying to get this system into Core, I am giving up now. I will continue working on it in Contrib, possibly, but I think this is too controversial to ever make it into Core, and I am tired of begging people to get patches reviewed.

Thanks to everyone who did offer constructive suggestions, though! Much appreciated. The contrib module (if I release it as contrib) will be much better due to your help.

Oh, and if anyone is interested in co-maintaining the Configurable Help contrib module, contact me in the issue queue there (links to the current sandbox are in the issue summary).

For anyone still following this closed issue...

I have restarted this initiative. There are two main issues:
#2592487: [meta] Help system overhaul is in the Ideas queue. Currently it is a Proposed Plan. We are waiting for sign-off from Product and Release managers to make it an official Community Initiative.

Meanwhile, #2920309: Add experimental module for Help Topics is working on a Core patch (probably premature but oh well). We're doing this by filing individual issues in the Sandbox module, so that we have a way to discuss individual changes to the Core patch. Here's the issue queue:
https://www.drupal.org/project/issues/2369943

If you'd like to officially be part of the team for this initiative, please go comment on the Ideas queue issue, and/or jump in and review patches on Sandbox issues. Meanwhile, I've gone through this issue and compiled a list of people who helped here with patches/reviews/tests and hopefully everyone will get credited if we actually get this into Core. Thanks!!