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

Parser structure #939

Closed
hzlmn opened this issue Dec 29, 2016 · 13 comments
Closed

Parser structure #939

hzlmn opened this issue Dec 29, 2016 · 13 comments

Comments

@hzlmn
Copy link
Contributor

hzlmn commented Dec 29, 2016

While was reading throught postcss sources, found a moment that current parsing architecture looks

 ___________                        __________
|           |                      |          |
| Tokenizer | -> List of Tokens -> |  Parser  | -> Parser eats them and produces AST
|___________|                      |__________|

While it works fine, I am really wondering if there are any reasons for not using tokens streaming like babylon do for example, where you plug tokenizer in parser, or expose something like readNextToken?

In this manner you only scan source once, instead of scanning source and then iterating over tokens, that could provide significant performance boost.

@ai
Copy link
Member

ai commented Dec 29, 2016

There is no real reason for it. Personally, I really like this idea, especially after csstree show it’s great parser with nextToken idea (with many other great ideas).

Do you want to update current parser and tokenizer? Or you could take csstree parser and try to connect it to PostCSS’ parser.

@ai ai added the enhancement label Dec 29, 2016
@hzlmn
Copy link
Contributor Author

hzlmn commented Dec 29, 2016

I could do both, it's IMO a question to you and core team, what will be better for further plans.

@ai
Copy link
Member

ai commented Dec 29, 2016

@hzlmn I think you could start from backporting csstree tokenizer. If it possible, it will give a tokens streaming in the fastest way. If it is not possible we could try to add this feature to current tokenizer.

@lahmatiy do you think it is possible to backport csstree tokenizer to PostCSS?

@hzlmn
Copy link
Contributor Author

hzlmn commented Dec 29, 2016

@ai Ok, I gonna read throughcsstree source to figure out what could be done.

@hzlmn
Copy link
Contributor Author

hzlmn commented Dec 30, 2016

@ai As far as i can see, it is pretty possible to backport tokenizer from csstree. I started work on it and will create WIP pull request as soon as i can for ealier feedback.

@ai
Copy link
Member

ai commented Dec 30, 2016

Awesome! 😍🎅 Feel free to ask any help.

@hzlmn
Copy link
Contributor Author

hzlmn commented Dec 30, 2016

@ai Seems like project' folder is getting bigger, what do you think about decoupling parser, tokenizer structures to separate folders? It could close one spot here #503 .

@ai
Copy link
Member

ai commented Dec 30, 2016

Could you show some structure example?

But I think separating set and parser is a good thing.

@tunnckoCore
Copy link

tunnckoCore commented Dec 30, 2016

Externalizing tokenizer to separate package is even better thing. :D Such tokenizers works well for JSON parsing too. :)

Just seen csstree and will look its codebase.

@ai
Copy link
Member

ai commented Dec 30, 2016

@tunnckoCore you could write much better tokenizer for JSON only format :).

@hzlmn
Copy link
Contributor Author

hzlmn commented Jan 4, 2017

@ai Nevermind, anyway it should be part of another PR.

Quick question, does current parser support passing ignoreErrors param, because as I can see here , this option never passed ?

@ai
Copy link
Member

ai commented Jan 4, 2017

I use this option in Safe Parser.

@ai
Copy link
Member

ai commented Apr 14, 2017

Done in orias branch

@ai ai closed this as completed Apr 14, 2017
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