Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(prettier): Ignore implicit closing bracket in rtc element #2592

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

bsmth
Copy link
Member

@bsmth bsmth commented Aug 17, 2023

Slightly ugly workaround to get tests to pass in #2576

This is the only case of a magic comment necessary, I think we can allow one exception for this.

@bsmth bsmth requested a review from a team as a code owner August 17, 2023 11:03
@bsmth bsmth requested review from dipikabh and removed request for a team August 17, 2023 11:03
@queengooborg
Copy link
Collaborator

Is the point of the test to demonstrate that there's an implicit closing bracket...?

(Side note, I can't find where this example is referenced in content...)

@bsmth
Copy link
Member Author

bsmth commented Aug 31, 2023

(Side note, I can't find where this example is referenced in content...)

It's used here: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/rtc

Is the point of the test to demonstrate that there's an implicit closing bracket...?

No, Prettier throws an error if there's a closing </rtc> regardless of the inclusion of an ignore comment:

["ERROR" - 10:29:11 AM] Unexpected closing tag "rtc". It may happen when the tag has already been closed by another tag. For more info see https://www.w3.org/TR/html5/syntax.html#closing-elements-that-have-implied-end-tags (12:5)
  10 |     <rtc xml:lang="en" style="ruby-position: over;">
  11 |         <rp>(</rp><rt>Malaysia</rt><rp>)</rp>
> 12 |     </rtc>
     |     ^^^^^^
  13 | </ruby>
  14 |

Note this is also valid and passes npm test:

<rtc xml:lang="en" style="ruby-position: over;"/>
        <rp>(</rp><rt>Malaysia</rt><rp>)</rp>

If we remove the Prettier ignore and let the formatter do it's thing it looks like this:

<ruby xml:lang="zh-Hant" style="ruby-position: under">
  <rbc>
    <rb></rb><rp>(</rp><rt></rt><rp>)</rp> <rb></rb><rp>(</rp><rt>lái</rt><rp>)</rp> <rb>西</rb><rp>(</rp
    ><rt></rt><rp>)</rp> <rb></rb><rp>(</rp><rt></rt><rp>)</rp>
  </rbc>

  <rtc xml:lang="en" style="ruby-position: over" />
  <rp>(</rp><rt>Malaysia</rt><rp>)</rp>
</ruby>

Note that this element is considered deprecated so I don't think it's necessary to worry over adding special handling here for tests to pass.

@caugner
Copy link
Contributor

caugner commented Sep 7, 2023

@bsmth Maybe add live-examples/html-examples/inline-text-semantics/rtc.html to the .prettierignore file instead to keep it simple?

@bsmth bsmth requested a review from a team as a code owner September 7, 2023 16:27
@bsmth
Copy link
Member Author

bsmth commented Sep 7, 2023

@bsmth Maybe add live-examples/html-examples/inline-text-semantics/rtc.html to the .prettierignore file instead to keep it simple?

Yeah that's fair, done in 43e6cc6

@caugner caugner changed the title chore: Ignore implicit closing bracket in rtc element chore(prettier): Ignore implicit closing bracket in rtc element Sep 26, 2023
@caugner caugner merged commit 213da5c into mdn:queengooborg-patch-1 Sep 26, 2023
caugner added a commit that referenced this pull request Oct 10, 2023
* Run all linting actions in CI workflow

* Run Prettier

* chore(prettier): Ignore implicit closing bracket in rtc element (#2592)

* chore: Ignore implicit closing bracket in rtc element

* chore: add rtc element example to .prettierignore

* style(prettier): ignore CHANGELOG

* chore: revert CHANGELOG.md

* style: run `prettier -w .`

---------

Co-authored-by: Brian Thomas Smith <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
@bsmth bsmth deleted the rtc-element-formatting branch October 18, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants