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

Fix hyphen html comment syntax highlight overflow #116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wmcmurray
Copy link

Currently, when an html comment like this one is used: <!---------- Sample Comment -----------> (no spaces between the hyphens forming the line and those forming the comment delimiter), everything under it will be grayed.

I looked at HTML's comments RFC specs to verify if a comment like this is illegal or not, but it seems good to me, so I copied the same value used in language-html to make sure it's consistent, and it fixed the issue.


Also fun fact, Github's syntax highlight also seem to suffer this same problem ! 🤔 🤯 !

Here is an HTML syntax highlight: 👍🏼

<template lang="html">
  <!---------- Sample Comment ----------->
  <p>blablabla</p>
</template>

<script>
export default {}
</script>

and here is a Vue syntax highlight: 👎🏼

<template lang="html">
  <!---------- Sample Comment ----------->
  <p>blablabla</p>
</template>

<script>
export default {}
</script>

@confused-Techie
Copy link
Collaborator

While from my personal understanding of the spec you linked would say be Each comment starts with --' and includes all text up to and including the next occurrence of --'. meaning that <!---- in this snippet the second set of -- would determine the end of the comment, since it's the next occurrence, if we look elsewhere in Atom/Pulsar, where the TextMate HTML grammar accepts your entire example comment as a comment, as well as the TreeSitter HTML grammar, I'm willing to side with the other grammars in Pulsar/Atom and assume this must be valid and my interpretation is instead wrong.

Although, realistically, HTML is a very non-strict standard, where most browsers make a best effort to accept all HTML, even if invalid. Resulting in many pages online being technically invalid, but still achieving the results intended.


With all of that said, thanks a ton for contributing! I know it's taken some time to getting this PR reviewed, and I've since pushed some updates that do automatic testing, to see how this PR behaves. But I'll go ahead and as soon as I can provide a review on the content of your code as well. Appreciate your time!

@confused-Techie
Copy link
Collaborator

So was able to test this out, and seems your PR isn't behaving the way it's intended to.

image

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.

2 participants