-
Notifications
You must be signed in to change notification settings - Fork 44
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
Fix token location computing #124
Conversation
a6bfe1a
to
bba7301
Compare
tests/python/test_parser.py
Outdated
self.assertEqual(string1.token_last.location.end.line, 3) | ||
self.assertEqual(string1.token_last.location.end.column, 27) | ||
|
||
string2 = rule.strings[1] | ||
self.assertEqual(string2.identifier, '$2') | ||
self.assertEqual(string2.location.begin.line, 4) | ||
self.assertEqual(string2.location.begin.column, 10) | ||
# self.assertEqual(string2.location.end.line, 4) # FIXME: 4 != 8 | ||
# self.assertEqual(string2.location.end.column, 40) # FIXME: 2 != 40 | ||
self.assertEqual(string2.location.end.line, 4) # FIXME: 4 != 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove FIXME
comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
- I tested your changes and the locations of the strings (plain, hex) are correct
@@ -21,13 +22,19 @@ class Location | |||
*/ | |||
struct Position { | |||
|
|||
Position() : line(1), column(0) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be beneficial to add comment about how we count lines and columns.
- lines from 1
- columns from 0
Fixing the location of tokens of strings.