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

Improved parser error messages #261

Closed
wants to merge 1 commit into from
Closed

Improved parser error messages #261

wants to merge 1 commit into from

Conversation

Razican
Copy link
Member

@Razican Razican commented Feb 25, 2020

Parser error messages now also include the line and the column where the offending lexer tokens started. This should make it easier to debug JavaScript code, and also the limitations of the engine itself.

I also took the opportunity to idiomatize some derives. Since derives
are part of the definition of a structure/type, they should go with
the definition, and not with the comments.

Copy link
Member

@jasonwilliams jasonwilliams left a comment

Choose a reason for hiding this comment

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

Looks good!, just needs a rebase
p.s whats with the moving of attributes and comments?

I also took the opportunity to idiomatize some derives. Since derives
are part of the definition of a structure/type, they should go with
the definition, and not with the comments.
@Razican Razican closed this Mar 7, 2020
@Razican Razican reopened this Mar 7, 2020
@Razican
Copy link
Member Author

Razican commented Mar 7, 2020

Hi, I re-based to master.
The change on comments/derives was to make them more idiomatic. The #[derive] attribute is an attribute of the struct/enum whatever, not on the comment, so even if it works the other way, it separates functionality wit some comments, and the comments are also related to the derived trait.

Checking the official documentation, this is the common practice: https://doc.rust-lang.org/rust-by-example/trait/derive.html

@jasonwilliams
Copy link
Member

Not ignoring this PR, I’m just working on refactoring the parser right now, so some of these are on hold

@jasonwilliams
Copy link
Member

jasonwilliams commented Mar 27, 2020

Closing as superseeded by jasonwilliams#283

jasonwilliams pushed a commit that referenced this pull request Mar 27, 2020
* Ported #261 to the new parser

* Fixed some comments

* More comment improvements

* Fixed one error in the parser, and improved error messages

* Fix the `next_if` function

Also, update documentation.

Co-Authored-By: HalidOdat <halidodat@gmail.com>

* Improved one particular error

* Fixed function declaration parsing

Co-authored-by: HalidOdat <halidodat@gmail.com>
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.

None yet

2 participants