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

[Tracking issue] Document initialStates #99

Closed
RReverser opened this issue Jul 20, 2017 · 6 comments
Closed

[Tracking issue] Document initialStates #99

RReverser opened this issue Jul 20, 2017 · 6 comments

Comments

@RReverser
Copy link
Contributor

Currently initialStates are just told to be "strings, each being the name of a tokenizer state" and only ["data state"] is mentioned (as a default, it doesn't appear explicitly in tests).

Such definition is a bit vague for consumers that want to use the provided test format.

@RReverser
Copy link
Contributor Author

This is especially important because changes / new additions currently arrive silently (e.g. #92 introduced CDATA section state) and there is no clear list of states that must be exposed.

@inikulin
Copy link
Member

I guess the best solution will be to use state names as they are declared in spec. And then just point to the spec in the README for the comprehensive list of possible state names.

@RReverser
Copy link
Contributor Author

RReverser commented Jul 21, 2017

@inikulin That could work but tests use only few of these states, exposing every one of them would be quite inconvenient (if not impossible) for many parsers. I'd prefer an explicit list in README that would be updated if any new entry state is added.

@gsnedders
Copy link
Member

gsnedders commented Jul 21, 2017

Yeah, I agree with @RReverser. Especially if someone reviews #83 and we can build some lint for this! I think all we need are the states the tree constructor moves us to, as everything else should be reachable from one of those?

@gsnedders
Copy link
Member

And yes, I believe they should just use the names as declared in the spec. I think we at some point just used lowercased spec names (for… reasons?).

@RReverser
Copy link
Contributor Author

I think we at some point just used lowercased spec names (for… reasons?).

Yeah, it was one of implicit breaking changes that led me to creating this issue.

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

No branches or pull requests

3 participants