Skip to content

Parse Patterns and Comments line-by-line #160

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

Closed
wants to merge 2 commits into from

Conversation

stasm
Copy link
Contributor

@stasm stasm commented Aug 3, 2018

Based on #159, this is a refactor of the grammar to a more line-by-line-based approach. This is a good refactor: it greatly simplifies the EBNF and moves much of logic related to creating the AST into abstract.mjs. This means that the EBNF can be used more effectively now for its primary purpose: validating whether something is correct Fluent or not.

@stasm stasm force-pushed the line-by-line branch 2 times, most recently from bd433b0 to 89af2cc Compare August 6, 2018 09:40
@stasm stasm mentioned this pull request Aug 8, 2018
@Pike
Copy link
Contributor

Pike commented Aug 15, 2018

From an implementers POV, I think that the EBNF and grammar.mjs should inform what parser authors should do. abstract.mjs comes in handy when needed, but I'd discourage to require implementers to have it in mind in all grammar productions. That's going to make it tedious to get implementations right.

the EBNF is not where we define where spans start and end.
#163 (comment)

I strongly disagree, to me that would resemble conflicting information within the fluent specification.

@stasm
Copy link
Contributor Author

stasm commented Aug 16, 2018

I strongly disagree, to me that would resemble conflicting information within the fluent specification.

I think there's a misunderstanding between us on what the roles of grammar.mjs and abstract.mjs are. Let me try to summarize my take:

The purpose of the EBNF (and grammar.mjs from which the EBNF is generated) is to describe the grammar of the Fluent language. The purpose of the grammar is to be able to verify if any given text is a well-formed Fluent document. Defining the spans of AST nodes is a non-goal for the grammar because the grammar doesn't describe the AST. It describes the CST, which is transformed into an AST by the transformations in abstract.mjs. The sum of grammar.mjs and abstract.mjs is what constitutes the Fluent spec. Both are equally important in producing an AST. grammar.mjs defines the rules of well-formed-ness. abstract.mjs defines the rules of validity.

Is this how you understand these roles as well?

@Pike
Copy link
Contributor

Pike commented Aug 16, 2018

I guess that for tooling, only the CST is important. Tools need to be able to pinpoint to a particular place in the code. That's also where spans are important.

The AST as an abstraction of the CST is interesting for runtime. And to some extent for higher-level checks, if there's a mapping back to the CST. So, the concrete syntax has spans and stuff, whereas its abstraction doesn't. The reason that our AST does is merely that we don't have a CST to start with.

The more code is in abstract.mjs, the harder it's going to be to write powerful tooling for Fluent.

@stasm
Copy link
Contributor Author

stasm commented Aug 16, 2018

No, the CST is useful only for the purpose of generating the AST. Tools should definitely use the AST.

@stasm
Copy link
Contributor Author

stasm commented Aug 22, 2018

The ease the review process, I've split this PR into two: one about Comments (#168) and the other one about Patterns (#169).

@stasm stasm closed this Aug 22, 2018
@stasm stasm deleted the line-by-line branch August 22, 2018 14:55
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