Problem/Motivation

In order to assist with getting help_topics into the beta stage, an internal and final Drupal\help_topics\FrontMatter class was added in #3069109: Replace help_topic meta tags with front matter.

This has now been added to Core. #3064854: Allow Twig templates to use front matter for metadata support

So we need to use the new Core class in Help Topics and get rid of the one we added to Help Topics.

Proposed resolution

Use the new Core class in Help Topics and get rid of the one we added to Help Topics.

Remaining tasks

Patch, review.

User interface changes

None

API changes

The existing help topics FrontMatter class will go away. Help Topics is Experimental, so this is OK to do.

Data model changes

None

Release notes snippet

None, this is an experimental module.

CommentFileSizeAuthor
#9 3085972-9.patch8.02 KBandypost
#9 interdiff.txt2.01 KBandypost
#8 3085972-8.patch6.54 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markcarver created an issue. See original summary.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Issue summary: View changes

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Title: [PP-2] Replace Drupal\help_topics\FrontMatter with Drupal\Component\Utility\FrontMatter » [PP-1] Replace Drupal\help_topics\FrontMatter with Drupal\Component\Utility\FrontMatter

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Title: [PP-1] Replace Drupal\help_topics\FrontMatter with Drupal\Component\Utility\FrontMatter » Replace Drupal\help_topics\FrontMatter with Drupal\Component\Utility\FrontMatter
Status: Postponed » Active

#3064854: Allow Twig templates to use front matter for metadata support was committed.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Looks it still blocked on #3075427: Create TemplateDiscovery for plugin managers to use

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Active » Needs review
FileSize
3085972-8.patch6.54 KB

Let's see how it works (I see small difference in regexp in FM classes)

// component
  const REGEXP = '/\A(' . self::SEPARATOR . '(.*?)?\R' . self::SEPARATOR . ')(\R.*)?\Z/s';
// help topics
  const FRONT_MATTER_REGEXP = "{^(?:" . self::FRONT_MATTER_SEPARATOR . ")[\r\n|\n]*(.*?)[\r\n|\n]+(?:" . self::FRONT_MATTER_SEPARATOR . ")[\r\n|\n]*(.*)$}s";
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
interdiff.txt2.01 KB
3085972-9.patch8.02 KB

changing namespace is not enough, now it should work (tests pass locally)

The last submitted patch, 8: 3085972-8.patch, failed testing. View results

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community

This patch (a) removes the now-extraneous class in Help Topics (b) switches us to use the new Component class and (c) makes a few small syntax changes between our local class and the new Component class so it can be used.

This is straightforward and the tests pass, with no coding standards. Let's do it!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs followup, +Needs issue summary update

The other parts of the issue summary need to be moved to their own follow-ups then...

Are not done here.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue summary: View changes
Related issues: -#3068483: Automatically map !translate YAML tag to TranslatableMarkup

Right, sorry about that! I split off the one about the Discovery class, and added notes/issues to the Roadmap #3027054: Help Topics module roadmap: the path to beta and stable.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs work » Reviewed & tested by the community
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Issue tags: -Needs followup, -Needs issue summary update

The follow-up is #3176735: Replace Drupal\help_topics\HelpTopicDiscovery with core/lib Twig discovery class

Help topics is not stable yet, so we can't mix changes in it with other core changes

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed a77d83e and pushed to 9.1.x. Thanks!

  • alexpott committed a77d83e on 9.1.x 
    Issue #3085972 by andypost, jhodgdon: Replace Drupal\help_topics\...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.