filter_autop should ignore twig.config debug html comments

Tests showing this is failing with:

<p>Text here
<!-- THEME DEBUG -->
<!-- THEME HOOK: 'html' -->
<!-- FILE NAME SUGGESTIONS:
* html--node.html.twig
x html.html.twig -->
<span>Test</span></p>

Results in:

<p>Text here</p>
<!-- THEME DEBUG -->
<!-- THEME HOOK: 'html' -->
<!-- FILE NAME SUGGESTIONS:
* html--node.html.twig
x html.html.twig -->
<p><span>Test</span></p>

Note that when you have twig debug enabled, line breaks are always added before and after the nested twig template, even with {% apply spaceless %}{% end apply %}

Okay this handles it now. Avoided regex here as its getting overly complicated given the regex would need to work across lines. Figured it is more straightforward and readable to check subsequent and preceding lines.

To test this out, without change to filter.module, this now fails:

./vendor/bin/phpunit -c core/phpunit.xml --debug core/modules/filter/tests/src/Kernel/FilterKernelTest.php --filter=testLineBreakFilter

With THE change to filter.module, it passes.

Makes sense!

Note that only the text format uses this by default. If you're using CKEditor 5, you should not be using filter_autop at all, and you can safely disable it.

Thanks for doing this! :)

Thanks for the quick review! Applied your suggestions, looks like this is ready to go then. Thanks!

Updating issue summary to mark all as completed as well (forgot!)

Needs updating after ✨ Make it more obvious that a Twig template is overridden Fixed got in

Thanks for flagging. I updated the filter to cover that now + added an additional bit to the test.

Can the MR be rebased please

I think I did grab all from 11.x and merge it in. Isn't the squashing of commits happening when this eventually gets merged in? Probably I am not understanding what you are asking @smustgrave sorry :)

Appears to be failing unit tests but have seen that resolved with a rebase.

Ah I see, thanks!

Seeing green now

Ran the test-only feature https://git.drupalcode.org/issue/drupal-3421843/-/jobs/995505 and it failed as expected.

Appears feedback has been addressed with regards to #11.

Looks good!

Ah nice, handy feature; saw that 'Downstream failure' and got curious. Thanks for jumping in and helping get this one ready again!

One question on the MR.

@catch added a suggested route instead of making it more generic - what dok you think?

@scott_euser seemed to got the thumbs up for that approach

Did a rebase and that unit failure went away. Seen that on a few slightly older tickets.

From what I can tell feedback has been addressed and the update didn't break the changes added in ✨ Make it more obvious that a Twig template is overridden Fixed

nod_ → closed merge request !6640

Committed 6ac4200 and pushed to 11.x. Thanks!

Tests fail on 10.2.x, won't backport there. Please open a 10.2.x MR for it if it's important to have on that version.

Great thank you both!

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