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

formatters examples appear as html-encoded text #16676

Closed
mdjermanovic opened this issue Dec 16, 2022 · 7 comments · Fixed by #16669 or #16719
Closed

formatters examples appear as html-encoded text #16676

mdjermanovic opened this issue Dec 16, 2022 · 7 comments · Fixed by #16669 or #16719
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects

Comments

@mdjermanovic
Copy link
Member

See https://eslint.org/docs/latest/user-guide/formatters/

For example, the checkstyle example used to look like this:

image

But now it looks like this:

image

By checking deploy previews on PRs, it seems that this change was introduced by #16606

@mdjermanovic mdjermanovic added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion labels Dec 16, 2022
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Dec 16, 2022
@amareshsm
Copy link
Member

let me check this issue.

@amareshsm amareshsm self-assigned this Dec 17, 2022
@chenxsan
Copy link
Contributor

chenxsan commented Dec 18, 2022

When using text language, we were actually opting-out of 11ty syntax highlighting. But I don't think that's the standard way to handle it.

Here's how GitHub renders text language https://github.com/eslint/eslint/blob/main/docs/src/user-guide/formatters/index.md:

And this is how markdown-it renders it:

So I think it maybe more reasonable to decode the code block in the markdown source code instead of hacking on the new syntax highlighter with non-standard trick. Another benefit would be more readable code block.

@mdjermanovic
Copy link
Member Author

So, before #16606 contents of text blocks were not being html-escaped during the generation of html files from markdown, but now they are? In that case, it seems that we should skip html-escaping the formatters' output when generating the markdown file.

<%= formatterResults[formatterName].result %>

- <%= formatterResults[formatterName].result %>
+ <%- formatterResults[formatterName].result %>

@amareshsm
Copy link
Member

As @chenxsan mentioned

So I think it maybe more reasonable to decode the code block in the markdown source code instead of hacking on the new syntax highlighter with non-standard trick. Another benefit would be more readable code block.

Can we do this? @mdjermanovic thoughts?

@chenxsan can we fix this issue as part of #16669 PR itself?

@chenxsan
Copy link
Contributor

So, before #16606 contents of text blocks were not being html-escaped during the generation of html files from markdown, but now they are?

Exactly, actually I can found some issues due to not being html-escaped before #16606. Here's a preview page from #16605,

And here's the source code:

As you can see, texts like <relative/path/to/filename> are missing in the rendered result before #16606.

And here's the rendered version of #16606:

CleanShot 2022-12-28 at 10 36 44@2x

Back to this formatters' examples, I believe you're right. We no longer need to escape them here formatter-examples.md.ejs. I've pushed a fix in #16669

@mdjermanovic
Copy link
Member Author

Back to this formatters' examples, I believe you're right. We no longer need to escape them here formatter-examples.md.ejs. I've pushed a fix in #16669

Is this problem related to #16669? If not, it would be better to fix this in a separate PR.

@chenxsan
Copy link
Contributor

chenxsan commented Dec 28, 2022

Back to this formatters' examples, I believe you're right. We no longer need to escape them here formatter-examples.md.ejs. I've pushed a fix in #16669

Is this problem related to #16669? If not, it would be better to fix this in a separate PR.

More like an issue should be caught and fixed in #16606, though #16669 is a follow-up of #16606. I think it's better to fix in a separate PR.

Triage automation moved this from Needs Triage to Complete Dec 28, 2022
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jun 27, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jun 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects
Archived in project
Triage
Complete
3 participants