Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Fix Unicode and EOF positions #123

Merged
merged 1 commit into from
May 31, 2019
Merged

Fix Unicode and EOF positions #123

merged 1 commit into from
May 31, 2019

Conversation

dennwc
Copy link
Member

@dennwc dennwc commented May 30, 2019

Enable conversion from UTF-16 code points to bytes. Fixes #122. Also fixes end-of-file positions.

To prevent regressions, enable token verification for string literals and simple identifiers.

Signed-off-by: Denys Smirnov denys@sourced.tech


This change is Reviewable

@dennwc dennwc requested review from creachadair and bzz May 30, 2019 00:26
@dennwc dennwc self-assigned this May 30, 2019
…h#122

Signed-off-by: Denys Smirnov <denys@sourced.tech>
Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 204 files at r1.
Reviewable status: 1 of 205 files reviewed, 2 unresolved discussions (waiting on @bzz, @creachadair, and @dennwc)


driver/fixtures/fixtures_test.go, line 37 at r1 (raw file):

		},
	},
	VerifyTokens: []positioner.VerifyToken{

👍 I wonder, what is the reason for us not to have this enabled for all the types?


fixtures/_integration.java.legacy, line 1 at r2 (raw file):

CompilationUnit {

Could somebody please remind, what is .legacy used for?

Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 204 files at r1.
Reviewable status: 2 of 205 files reviewed, 2 unresolved discussions (waiting on @bzz, @creachadair, and @dennwc)

Copy link
Member Author

@dennwc dennwc left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 205 files reviewed, 2 unresolved discussions (waiting on @bzz and @creachadair)


driver/fixtures/fixtures_test.go, line 37 at r1 (raw file):

Previously, bzz (Alexander) wrote…

👍 I wonder, what is the reason for us not to have this enabled for all the types?

It only works for types with tokens.


fixtures/_integration.java.legacy, line 1 at r2 (raw file):

Previously, bzz (Alexander) wrote…

Could somebody please remind, what is .legacy used for?

It's an integration test for the Parsing API v1. Soon we will have a chance to drop it.

Copy link
Contributor

@creachadair creachadair left a comment

Choose a reason for hiding this comment

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

Reviewed 202 of 204 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @bzz)

Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dennwc dennwc merged commit ecfc603 into bblfsh:master May 31, 2019
@dennwc dennwc deleted the fix_pos branch May 31, 2019 14:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node position offset is not bytes
3 participants