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

Test comments in lexer.rs #47

Closed
jasonwilliams opened this issue Jul 1, 2019 · 9 comments
Closed

Test comments in lexer.rs #47

jasonwilliams opened this issue Jul 1, 2019 · 9 comments
Assignees
Labels
good first issue Good for newcomers Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com

Comments

@jasonwilliams
Copy link
Member

jasonwilliams commented Jul 1, 2019

lexer.rs now has a test suite, but testing comments are still missing.

Test would ensure single-line and multi-line comments are skipped passed and don't generate any tokens

Contributing

https://github.com/jasonwilliams/boa/blob/master/CONTRIBUTING.md

@akshay5995
Copy link

@jasonwilliams I'll take this up :)

@jasonwilliams jasonwilliams added the Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com label Sep 3, 2019
@jasonwilliams
Copy link
Member Author

@akshay5995 you still interested in taking this?

@Emanon42
Copy link
Contributor

Emanon42 commented Oct 4, 2019

I am interested in this. Can I take a try?

@Emanon42
Copy link
Contributor

Emanon42 commented Oct 5, 2019

#134

@Emanon42
Copy link
Contributor

Emanon42 commented Oct 5, 2019

CI failed. I read the lexer code and find out that it actually produces a comment token object which contains the comment string. But you said "single-line and multi-line comments are skipped passed and don't generate any tokens", is that a bug or feature?

@Emanon42
Copy link
Contributor

Emanon42 commented Oct 7, 2019

dont get reply yet, I assume this is feature and change tests.

@jasonwilliams
Copy link
Member Author

its a feature, we don't need to generate tokens for comments, they should be ignored.

@jasonwilliams
Copy link
Member Author

jasonwilliams commented Oct 7, 2019

if CI fails you can run whatever it does locally

cargo fmt --all -- --check

is what failed, so looks like formatting issues

@Emanon42
Copy link
Contributor

Emanon42 commented Oct 8, 2019

#134 has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com
Projects
None yet
Development

No branches or pull requests

3 participants