Closed (fixed)
Project:
Translation Template Extractor (potx)
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
8 Oct 2019 at 23:44 UTC
Updated:
17 Nov 2020 at 11:27 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
larowlanNote that the discovery marks them as translatable
so perhaps we don't need to do anything here?
Comment #3
andypostIt needs work to fix test at least
Comment #4
andypostThe commit for reference https://git.drupalcode.org/project/potx/commit/6e9bd50
Comment #6
jhodgdonRE #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.
Comment #7
andyposthere's parser change, I did copy of meaningful parts (the front matter parsing yml only from start of file)
Comment #8
andypostrestore summary
Comment #9
jhodgdonPatch looks good! Tests pass!
Some thoughts:
a) Nitpick:
This comment at the top of the function needs an update to say "front-matter data" instead of "meta tags".
b)
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:
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.
Comment #10
jhodgdonI 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.
Comment #11
andypostFix #9 (a,d)
Comment #12
andypostworking patch and interdiff
Comment #13
jhodgdonThat 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).
Comment #14
jhodgdonOfficially postponing and updating title/summary.
Comment #17
gábor hojtsyCrediting @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.
Comment #18
gábor hojtsyAs 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?
Comment #19
jhodgdonIn 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.
Comment #20
andypostre-queued for 8.x with required core
Comment #23
gábor hojtsySuperb, thanks.
Comment #24
jhodgdonGreat, 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?
Comment #25
gábor hojtsy@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).
Comment #27
jhodgdonFYI 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.
Comment #28
gábor hojtsyYay, thanks for working on this :)