Replace help_topic meta tags with front matter

This looks OK to me, if the parent issue gets done. A few notes and minor suggestions:
- I think the Twig service parameter to the HelpTopicDiscovery class should be added as 2nd not 1st parameter. The primary thing is the directories, normally, for plugin discovery classes.
- There is a @todo in there that doesn't have its own issue. If we are going this route, we'll need to create an issue to follow up with that [it's currently referencing #3068483: Automatically map !translate YAML tag to TranslatableMarkup but that issue wouldn't patch this @todo, so we need a separate issue].
- $root = \Drupal::root() -- that should be injected, not using \Drupal.
- There are a few typos in docs. I can get more specific if we're actually doing this.

That does sound like a good idea. I'll add the new issue to the Help Topics Roadmap issue.

Adding a proper issue summary

Over in #3027054: Help Topics module roadmap: the path to beta and stable @catch and @lauriii want to experiment with bring front matter into help topics only. So here's that patch. It uses @markcarver's code from #3064854: Allow Twig templates to use front matter for metadata support but implements this only for help topics.

Looks like the patch on the other issue has review comments from @alexpott that need to be addressed?

Gone through the module and updated all the references to meta tags and the documentation. And also changed the related key to be an array. Might as well use YAML's ability to provide data types.

As a note, this patch will need to be updated if this related issue's patch is committed.

@jhodgdon yeah well the other patch is the generic solution. The FrontMatter class in the patch is in the help topics module. The patch here happens to address all my comments on the other issue.

"happens to". :) Good.

Since there are multiple patches coming... Let us know when it's time to actually review this. Thanks!

@jhodgdon I'm done here - waiting for any reviews. Note I'm a bit nonplussed about front matter vs meta tags vs any other in--help-topic-twig-file solution. I wrote this patch because @lauriii and @catch wanted this explored prior to making help topics beta and as the person who introduced the meta tag solution I want to do my best to resolve this asap.

One thing that @catch mentioned is can we place the front matter in HTML comments? So if for some reason they did slip though twig parsing then they would do no harm.
So I guess that would be to replace --- with <!-- and --!>

This looks viable (plus, the tests pass). The tests and code both look reasonable to me.

I guess we need to test manually to verify that the meta-information YAML is not shown on the topic pages, or do we have a test for that? Probably should be a test if there isn't. Hm... It looks like some lines in HelpTopicTest pertaining to meta data were removed and not replaced with anything: core/modules/help_topics/tests/src/Functional/HelpTopicTest.php

-    // Use the article element to provide a positive assertion to improve the
-    // assertion that the help html does not contain meta tags.
     $this->assertContains('This is a theme provided topic.', $session->elementExists('css', 'article')->getText());
-    // Ensure that meta tags containing plugin information do not appear on
-    // topic pages
-    $session->elementNotExists('css', 'article meta');

Regarding putting the front matter into HTML comments... It seems like it might be an OK idea, but would that get it into the HTML of the topic pages? Not sure if Twig parsing outputs HTML comments within the template file -- I think it would? I don't think that is desirable. I guess we could remove it in the Discovery/Loader though?

+++ b/core/modules/help_topics/tests/src/Unit/HelpTopicTwigLoaderTest.php
@@ -51,6 +52,24 @@ public function testConstructor() {
+  /**
+   * @covers ::getSourceContext
+   */
+  public function testGetSourceContext() {
+    $source = $this->helpLoader->getSourceContext('@' . HelpTopicTwigLoader::MAIN_NAMESPACE . '/test.topic.html.twig');
+    $this->assertEquals('{% line 4 %}<h2>Test</h2>', $source->getCode());
+  }

@jhodgdon this test proves that the Twig source returned by the HelpTopicTwigLoader doesn't contain the front matter. I don't think we need a further test showing this doens't end up in HTML because we've proved the twig compiler doesn't get the front matter what so ever.

Re html comments @markcarver said

given that front matter is --- almost always (outside of core), I don't think it's wise to try and change expected behavior of front matter that way... it'll just be another DrupalWTF

I'm inclined to agree. If we're going with front matter because it's what other systems use then we should do it in the same way.

The patch #9 looks great. I think it would be great to have Help Topics use front matter from beta1 to avoid seeming like we're going back and forth on the API.

I know I've indicated on Slack and on #3064854: Allow Twig templates to use front matter for metadata support that I'm +1 to the front matter approach. This approach is used by some popular Twig based CMS's such as Jekyll, Hugo, and Grav. This is also more consistent with our pre-existing tooling since it's using YML.

+1 to #18 as well. I would rather go with an approach that's popularly used in the broader ecosystem rather than try to invent our own thing. In my opinion, it's better that these end up rendered visible because it could prevent accidentally leaking these into HTML, or at least it would make it more obvious (of course for both, users and developers).

Thank you @alexpott and @markcarver for driving this forward with this fast pace!

Seems fine to me. Thanks for the quick work so it didn't hold up Beta on the Help Topics module (although we will need to reroll one patch, or this one if the other gets in first, see comment #10 -- #3066512: Add checks for syntax and display of help topic Twig template files).

I've updated the issue summary to make the case for why we should do this.

1. +++ b/core/modules/help_topics/src/FrontMatter.php
   @@ -0,0 +1,173 @@
   +  public static function load($source, $serializer = '\Drupal\Component\Serialization\Yaml') {
   +    return new static($source, $serializer);
   

   not sure what this adds that you don't get from using the constructor 🤔

   edit: convenient chaining, without extra ()
   ✅

2. +++ b/core/modules/help_topics/src/HelpTopicPluginManager.php
   @@ -17,18 +17,20 @@
   + * The Twig file must contain YAML front matter with a key named 'label'. It can
   

   I think we should link to docs/homepage on front matter here, as it might be a new concept to devs, but that can be a followup as part of adding it in #3064854: Allow Twig templates to use front matter for metadata support

Committed 5691efb and pushed to 8.8.x. Thanks!

Nice work folks.

Just to retrospectively agree with #18 and #20, those are good reasons not to use HTML comments, I just wanted a reason not to in case that had been overlooked for some reason.

Created follow-up issue for POTX: #3086657: Parse new front-matter format for help topic titles
Also will add this to the Roadmap issue.