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.

Sign up for GitHub

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

Jump to bottom

Fixed bugs in Spec parser for terminals (#145) #146

Merged
merged 1 commit into from
May 27, 2023
Merged

Fixed bugs in Spec parser for terminals (#145) #146

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/scala/esmeta/spec/util/Parser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ trait Parsers extends LangParsers {

/** terminals */
lazy val term: Parser[Terminal] = {
"`[^`]+`|```".r ^^ {
"`[^`]+`|`[`]+`".r ^^ {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for grammarkdown don't mention this: https://github.com/rbuckton/grammarkdown#terminals

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few things (used by the ES spec) that the grammarkdown docs don't mention or aren't clear on. I think that's more of an issue for grammarkdown than for esmeta.

case str =>
Terminal(str.substring(1, str.length - 1))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't 1 be replaced with the length of the delimiter here?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would the length of the delimiter not be 1?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure what 4 or more backticks in a row is supposed to mean. It doesn't appear to be legal. So I don't know.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ecmarkup accepts 4 backticks and renders it as a literal terminal consisting of two backticks: 2418 preview

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reading the code for grammarkdown and it doesn't seem it would support 4 backticks in a row, though I could be misreading. Does it have something to do with tc39/ecma262#2418 using ` and the entities being processed by esmeta but not grammarkdown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESMeta unescapes the HTML entities before parsing the spec.html. Thus, the text ```` is unescaped as: ````, and ESMeta parses the four backticks (````) as a literal terminal consists of two backticks (``) because the first and last backtick is used to represent literal terminals. I supported this feature because tc39/ecma262#2418 defines the literal terminal consisting of two backticks. Is there any problem with this extension?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that should be fine then. I didn't know about the pre-processing of HTML entities by esmeta.

}
Expand Down