-
-
Notifications
You must be signed in to change notification settings - Fork 193
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
Breaking: syncing start/end with range (fixes #349) #461
Conversation
Sorry for the delay, I have update the code as suggested. Let me know if anything else is required to change. |
this[STATE].templateElements.forEach(templateElement => { | ||
const terminalDollarBraceL = this.input.slice(templateElement.end, templateElement.end + 2) === "${"; | ||
|
||
templateElement.start--; | ||
templateElement.end += (terminalDollarBraceL ? 2 : 1); | ||
}); |
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.
Can we add a comment about why we decided to modify start/end at this point instead of while finishing the node?
const ast = espree.parse("/* foo */ bar /* baz */", { | ||
range: true | ||
}); |
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.
Can we add another test like this for Program
, but without range: true
and loc: true
?
it("should output the same value for loc, range and start and end in templateElement", () => { | ||
const ast = espree.parse("`foo ${bar}`;", { | ||
ecmaVersion: 6 | ||
}); | ||
|
||
assert.strictEqual(ast.body[0].expression.quasis[0].start, 0); | ||
assert.strictEqual(ast.body[0].expression.quasis[0].end, 7); | ||
assert.strictEqual(ast.body[0].expression.quasis[1].start, 10); | ||
assert.strictEqual(ast.body[0].expression.quasis[1].end, 12); | ||
}); |
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 the title doesn't explain this test.
it("should output the same value for loc, range and start and end in templateElement with loc and range", () => { | ||
const ast = espree.parse("`foo ${bar}`;", { | ||
ecmaVersion: 6, | ||
loc: true, | ||
range: true | ||
}); | ||
|
||
assert.strictEqual(ast.body[0].expression.quasis[0].start, ast.body[0].expression.quasis[0].loc.start.column); | ||
assert.strictEqual(ast.body[0].expression.quasis[0].end, ast.body[0].expression.quasis[0].loc.end.column); | ||
assert.strictEqual(ast.body[0].expression.quasis[1].start, ast.body[0].expression.quasis[1].range[0]); | ||
assert.strictEqual(ast.body[0].expression.quasis[1].end, ast.body[0].expression.quasis[1].range[1]); | ||
}); |
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.
This test is a bit confusing, quasis[0]
start/end is compared to loc
only, while quasis[1]
to range
only.
@anikethsaha are you still working on this? |
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.
Changes look good. I'll address the remaining comments in a separate PR.
Syncing the range and loc with
start
andend
for bothprogram
andtemplateElement
nodes.