Skip to content
Snippets Groups Projects

Resolve #3421843 "FilterAutoP should ignore twig.config debug html comments"

Closed Resolve #3421843 "FilterAutoP should ignore twig.config debug html comments"
issue/drupal-3421843:3421843-filterautop-should-ignore into 11.x
1 unresolved thread
Closed Scott Euser requested to merge issue/drupal-3421843:3421843-filterautop-should-ignore into 11.x
1 unresolved thread

Closes #3421843

Merge request reports

Merge request pipeline #139203 passed with warnings

Stage: 🏗️ Build
Stage: 🪄 Lint
Stage: 🗜️ Test
Pipeline: drupal-3421843

#139205

    Merge request pipeline passed with warnings for 07a902a2

    Approval is optional
    Code Quality is loading
    Test summary results are being parsed
    Full report

    Closed by Théodore BiadalaThéodore Biadala 1 year ago (Apr 6, 2024 12:48pm UTC)

    Merge details

    • The changes were not merged into 11.x.

    Activity

    Filter activity
    • Approvals
    • Assignees & reviewers
    • Comments (from bots)
    • Comments (from users)
    • Commits & branches
    • Edits
    • Labels
    • Lock status
    • Mentions
    • Merge request status
    • Tracking
  • Wim Leers
  • Wim Leers
  • Scott Euser resolved all threads

    resolved all threads

  • Scott Euser resolved all threads

    resolved all threads

  • Scott Euser added 1 commit

    added 1 commit

    • fa23bbe9 - Apply 3 suggestion(s) to 1 file(s)

    Compare with previous version

  • Scott Euser resolved all threads

    resolved all threads

  • Scott Euser
    Scott Euser @scotteuser
    Author

    Applied suggestions, thanks!

  • Théodore Biadala
  • Scott Euser added 183 commits

    added 183 commits

    Compare with previous version

  • Scott Euser added 1 commit

    added 1 commit

    • 36adc146 - Update _filter_auto_p to account for updated theme debug markup

    Compare with previous version

  • Scott Euser added 38 commits

    added 38 commits

    Compare with previous version

  • catch @catch started a thread on the diff
    • core/modules/filter/filter.module
    734 734 }
    735 735 }
    736 736 elseif (!$ignore) {
    737 // Skip if the next chunk starts with Twig theme debug.
    738 // @see twig_render_template()
    739 if (isset($chunks[$i + 1]) && $chunks[$i + 1] === '<!-- THEME DEBUG -->') {
    • C
      catch @catch
      Maintainer

      This is very specific, is it worth adding some kind of generic comment format that _filter_autop() supports and then using that in the twig debug? I realise this inverts the dependency but it would be a soft dependency.

    • Scott Euser
      Scott Euser @scotteuser
      Author

      I was nervous here of catching anything that isn't specifically added by core. Eg if someone happens put a normal <!-- --> comment then a link break would we not still want to add the auto p? To avoid breaking anything basically. Really wanted this to just stop making local/dev/staging behave differently from live sites.

      One thing I suppose I could do is add a separate test that enables twig debug, renders a template programmatically, and strips it - if your concern basically is what if the twig debug code changes again? That way changing the twig debug code would cause this test to fail. Would that be sufficient rather than making this less specific?

    • Théodore Biadala
      Théodore Biadala @nod_
      Maintainer

      That sounds reasonable to me

    • Scott Euser
      Scott Euser @scotteuser
      Author

      Was a bit trickier than I expected to do it at the kernel test level, but wanted to keep it there to avoid it being a longer running test + keeping it near the other test covering filter auto p. Wondering if this is okay - if there is any change to the formatting for theme debug, this will fail the test, so the change from [#3420709] would have triggered this then.

    • Please register or sign in to reply
  • Scott Euser added 1 commit

    added 1 commit

    • 61264028 - Ensure tests continue to apply even if twig debug formatting changes

    Compare with previous version

  • Scott Euser added 1 commit

    added 1 commit

    Compare with previous version

  • Stephen Mustgrave added 268 commits

    added 268 commits

    • 54d133ce...7dd6b190 - 267 commits from branch project:11.x
    • 07a902a2 - Merge branch '11.x' of git.drupal.org:project/drupal into 3421843-filterautop-should-ignore

    Compare with previous version

  • Théodore Biadala closed

    closed

    • Please register or sign in to reply