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 lloc tracking for multi line blockrefs #2035
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Build SUCCESS |
furiel
approved these changes
May 15, 2018
Whops I forgot travis is failing before approve. When the style check problem is fixed, this pr is ready to be merged imo. |
For tokens that span multiple lines (in our case this only happens when a block-ref is parsed that spans multiple lines), we blew up the line number counter, so whenever reporting locations or errors we got an incorrect value. This patch fixes that by shifting to the new line number relative to last_line, instead of simply increasing the first_line. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
…vel configuration We are user friendly by pointing users to the mailing list address when reporting an error, however we did that even in cases where the error happened in a recursive grammar, possibly printing it multiple times. Avoid that by checking if we are indeed in the main parser. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
Even in the case the block ref parsing fails, report a location that spans the entire reference, just like in the true branch. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
Whenever we report a syntax error, we include a description of the current context. However this description was pretty concise a bit too much, this patch improves those to make the error message easier to understand. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
MrAnno
force-pushed
the
fix-lloc-tracking-for-multi-line-blockrefs
branch
from
May 19, 2018 21:41
1fa2234
to
a8f2636
Compare
MrAnno
approved these changes
May 19, 2018
I'll fix the style issue and merge, thanks for the review.
…On May 19, 2018 23:44, "László Várady" ***@***.***> wrote:
***@***.**** approved this pull request.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2035 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AArldh9DfMTTwpinukYl1tk_cU8YiI64ks5t0JJGgaJpZM4T-Owv>
.
|
I've fixed it for you :) |
Build SUCCESS |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue I noticed while finishing up a previous PR was that line tracking got broken by 2e196c3. While fixing the problem I've also improved error logging a bit more, which I've piggybacked to the same branch.