-
Notifications
You must be signed in to change notification settings - Fork 62
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: Account for BOM in the processor #282
Conversation
tests/processor.test.js
Outdated
it("should translate indented column numbers", () => { | ||
const result = processor.postprocess(messages); | ||
|
||
assert.strictEqual(result[2].column, 9); | ||
assert.strictEqual(result[3].column, 4); | ||
assert.strictEqual(result[4].column, 2); | ||
}); |
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 is the only test that was failing with BOM before the code change.
tests/processor.test.js
Outdated
it("should adjust fix range properties", () => { | ||
const result = processor.postprocess(messages); | ||
|
||
assert(result[2].fix.range, [185, 185]); | ||
assert(result[4].fix.range, [264, 265]); | ||
}); |
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'm not sure how, but this test was passing with BOM before the code change.
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.
Oh, this test was missing .deepStrictEqual
after assert
.
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.
Ok, in 68aede7 I fixed this test and added another one as per the repro from the original post. Both would fail without the code change.
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. Thanks!
The processor doesn't account for BOM, which would be passed in by ESLint if the file has it.
This causes incorrect adjustments of locations and fix ranges.
Repro: https://github.com/mdjermanovic/eslint-markdown-bom
In the above repro,
space-in-parens
rule fixes( a)
to( )
instead of(a)
.This PR updates
preprocess()
to remove BOM from the text it operates on.