On #3037229: Extract help topics label from meta tag in Twig files we set POTX up to parse meta-data from help topic Twig files.

On #3069109: Replace help_topic meta tags with front matter the format of the meta-data changed to use "front matter" instead of meta tags.

On #3064854: Allow Twig templates to use front matter for metadata support the parsing of the new front-matter is evolving... Once that is stable (this issue is postponed until then), we need to change POTX so that instead of parsing meta-tags in Twig files to get the topic titles, we instead parse the new front matter format.

Comments

jhodgdon created an issue. See original summary.

larowlan’s picture

Note that the discovery marks them as translatable

case 'label':
              $data[$key] = new TranslatableMarkup($value);
              break;

so perhaps we don't need to do anything here?

andypost’s picture

Status: Active » Needs review
StatusFileSize
new533 bytes

It needs work to fix test at least

andypost’s picture

Status: Needs review » Needs work

The last submitted patch, 3: 3086657-3.patch, failed testing. View results

jhodgdon’s picture

RE #2 -- both things are necessary. POTX will get them into the translation database on localize.drupal.org (this issue). Discovery will make it so that if a translation is in the local site's database, it is used to display the topic's title.

As you can see from the test update in #3, the POTX code from the previous POTX issue is not extracting the labels with the new Twig topic format, so we need a patch to make that happen.

andypost’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new2.18 KB
new2.22 KB

here's parser change, I did copy of meaningful parts (the front matter parsing yml only from start of file)

andypost’s picture

Issue summary: View changes

restore summary

jhodgdon’s picture

Patch looks good! Tests pass!

Some thoughts:

a) Nitpick:

 function _potx_parse_twig_file($code, $file, $save_callback) {
   // Extract meta tags, and save text in certain ones.

This comment at the top of the function needs an update to say "front-matter data" instead of "meta tags".

b)

// Implementation copied from core's \Drupal\help_topics\FrontMatter class.

That is correct today, but probably that class will be moving to core/lib soon. Not sure what to do about that...

c) A question: I noticed in the test template, we put the front matter ahead of the first comment in the Twig file (the part with the @file docs). Is that necessary, or can front matter be detected after the @file doc block?

d) In the code being replaced in this patch, we had:

-  $tags_to_save = array('help_topic:label');

This patch doesn't have a similar array at the top (which would make for an easier way to expand it to other translatable front matter keys, if we need that in the future). Maybe put that into the patch? Shouldn't be too difficult, and I think it makes the code more understandable.

jhodgdon’s picture

I answered my own question in #9 (c) -- the regular expression for detecting front matter starts with ^ - so it only detects front matter at the exact top of the Twig file. That is built into the front matter class as it currently exists in Core.

We should consider postponing this POTX issue until the Front Matter API is perhaps a bit more stable... a week or two? See these two related issues.

andypost’s picture

StatusFileSize
new1.08 KB
new2.39 KB

Fix #9 (a,d)

andypost’s picture

StatusFileSize
new1.08 KB
new2.39 KB

working patch and interdiff

jhodgdon’s picture

That looks good to me. However, the regular expression for detecting front matter in Drupal Core is evolving on #3064854: Allow Twig templates to use front matter for metadata support... We should probably postpone this issue until it is stable, and then come back and tweak the patch so that it matches what Drupal Core detects.

The second related issue I added in #10 is actually not relevant for POTX so I'm removing it from being related here (it is only a follow-up to use the results of #306454 in the help topics module).

jhodgdon’s picture

Title: Parse new front-matter format for help topic titles » [PP-1] Parse new front-matter format for help topic titles
Issue summary: View changes
Status: Needs review » Postponed

Officially postponing and updating title/summary.

gábor hojtsy’s picture

Title: [PP-1] Parse new front-matter format for help topic titles » Parse new front-matter format for help topic titles
Status: Postponed » Needs review

Crediting @ricovandevin and @herom from #3037229: Extract help topics label from meta tag in Twig files for their discussion about removing use of get_meta_tags() even if that was unrelated to converting to FrontMatter.

gábor hojtsy’s picture

As for feedback on this patch, are we ok with loosing support for extracting the meta tags? As in older core versions will not get their metatags extracted anymore, only the new versions using the new format?

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.31 KB
new2.33 KB

In my opinion, we should be fine with losing the meta tags support. It was pretty short-lived in the *experimental* Help Topics module in Core, and I don't think any other project started using that format (or even knew about it). Also, as noted on that other issue, it was problematic.

Let's use the latest patch for Front Matter from the other issue there, which was RTBC. From @andypost on #3037229-44: Extract help topics label from meta tag in Twig files; I marked both of these RTBC on comment 45.

andypost’s picture

re-queued for 8.x with required core

  • Gábor Hojtsy committed ee5db4b on 8.x-1.x
    Issue #3086657 by andypost, jhodgdon, ricovandevin, larowlan, herom:...

  • Gábor Hojtsy committed 21031a4 on 7.x-3.x
    Issue #3086657 by andypost, jhodgdon, ricovandevin, larowlan, herom:...
gábor hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Superb, thanks.

jhodgdon’s picture

Great, thanks! Could you possibly set off a new parsing job (when convenient with other commits etc.) on the last few 8.x/9.x releases of Core so we can get the help topic titles into the translation database?

gábor hojtsy’s picture

@jhodgdon: it would still need to be updated on and deployed to l.d.o. Usually @drumm takes that. I'll review the rest of the RTBC tasks in the project. (There do not seem to be any currently that would affect parsing, but I am not done yet).

Status: Fixed » Closed (fixed)

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

jhodgdon’s picture

FYI for anyone looking at this issue and wondering: I believe this has now been deployed on localize.drupal.org. I searched for a few strings that are titles of help topics on localize, and I found them all to be in the translation database as of Drupal core: 8.9.3 / 9.0.3 versions.

gábor hojtsy’s picture

Yay, thanks for working on this :)