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.

Sign up for GitHub

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

Jump to bottom

Use more verbose syntax error messages #694

Merged
merged 3 commits into from
Jan 14, 2021
Merged

Use more verbose syntax error messages #694

merged 3 commits into from
Jan 14, 2021

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Jan 10, 2021

Fixes #385

@Rangi42
Copy link
Contributor Author

Rangi42 commented Jan 10, 2021 •
edited
Loading

A problem: the different CI builds use different versions of bison, so some will define parse.error=detailed, parse.error=verbose, or neither. This causes different error messages that don't match the ones in the test .err files.

Edit: resolved by having separate .err and .simple.err files, with an initial rgbasm test to see which should be used.

@Rangi42 Rangi42 force-pushed the syntax-errors branch 4 times, most recently from 1bfb88b to 170ab8d Compare January 11, 2021 00:43
ISSOtm
ISSOtm requested changes Jan 11, 2021
View reviewed changes
Copy link
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

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

Great! You fixed the problems I encountered while I tried myself, and I find your solution to the "dependent syntax error" message acceptable.

src/asm/parser.y Outdated Show resolved Hide resolved
test/asm/test.sh Show resolved Hide resolved
@Rangi42 Rangi42 requested a review from ISSOtm January 11, 2021 14:09
ISSOtm
ISSOtm approved these changes Jan 14, 2021
View reviewed changes
CONTRIBUTORS.rst
@@ -40,6 +40,8 @@ Other contributors

- Quint Guvernator <quint@guvernator.net>

- Rangi <http://github.com/Rangi42>
Copy link
Member

Choose a reason for hiding this comment

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

...Honestly? You deserve it. (I should update my own emil, too :P)

src/asm/parser.y Show resolved Hide resolved
@ISSOtm ISSOtm merged commit a4ebb87 into gbdev:master Jan 14, 2021
@Rangi42 Rangi42 deleted the syntax-errors branch January 14, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ISSOtm ISSOtm ISSOtm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Syntax error" message is way too obtuse
2 participants
@Rangi42 @ISSOtm