Skip to content
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: handle windows crlf with more brutality #3

Merged
merged 2 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .gitattributes

This file was deleted.

9 changes: 3 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,11 @@ export interface DirEnt {

## Note regarding Windows

We're relying on text files and want to have byte-perfect representations of test fixtures, but this presents some problems for Windows. By default (unless the user has changed the settings), git on Windows will convert line endings to Windows style which include a carriage return (`\r`) character. This isn't good, because we don't know whether these characters are part of our fixture data or not!
Ideally when we're relying on text files for test data input we'd want to have byte-perfect representations of fixtures, but this presents some problems for Windows. By default (unless the user has changed the settings), git on Windows will convert line endings to Windows style which include a carriage return (`\r`) character. This isn't good, because we don't know whether these characters are part of our fixture data or not!

Our recommendation if you expect to support Windows users, or have Windows users try and run your tests, is to add a `.gitattributes` file to your Git project that uses testmark with something like the following:
So, testmark takes a rather brute-force approach to this problem and just strips out carriage return characters when they appear with a line-ending. In practice this _may_ impact the byte-perfect requirements for test fixtures, so you should be careful when using data that strays outside of standard printable character range, especially when control characters get involved. This is a text file format, if your data isn't text, then make it text by encoding in hex or base64 or something that reduces the character set to the safe range.

```
* text=auto
*.* text eol=lf
```
The `Document#original` property and `toString()` function are going to result in text that only uses UNIX line endings (`\n`). So even your input document will be transformed if it does a round-trip through testmark on Windows. Be warned!

## Note about the package name

Expand Down
5 changes: 4 additions & 1 deletion parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ export function parse (original) {
throw new TypeError('Expected a Markdown document string')
}

// sorry windows users, we're even turning your original to unix
original = original.replace(/\r?\n/g, '\n')

/** @type {Document & {original:string}} */
const doc = {
original,
lines: original.split('\n'), // can't split with \r? because we need offsets
lines: original.split('\n'),
dataHunks: /** @type {DocHunk[]} */ ([]),
hunksByName: new Map()
}
Expand Down
26 changes: 24 additions & 2 deletions test/test-parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,33 @@ const exampleMdExpectedHunks = [
]

describe('Read', () => {
it('can parse example.md', async () => {
/** @type {string} */
let exampleMdOriginal
let isWindows = false

before(async () => {
const exampleMd = new URL('../example.md', import.meta.url)
const exampleMdOriginal = await fs.promises.readFile(exampleMd, 'utf8')
exampleMdOriginal = await fs.promises.readFile(exampleMd, 'utf8')
isWindows = exampleMdOriginal.includes('\r\n')
})

it('can parse example.md', async () => {
const doc = parse(exampleMdOriginal)
assert.deepStrictEqual(exampleMdExpectedHunks, doc.dataHunks)
if (isWindows) {
assert.deepStrictEqual(toString(doc).replace(/\n/g, '\r\n'), exampleMdOriginal)
} else {
assert.deepStrictEqual(toString(doc), exampleMdOriginal)
}
})

it('can parse example.md as windows', async function () {
if (isWindows) { // skip test on windows
return this.skip()
}
const exampleMdOriginalWindows = exampleMdOriginal.replace(/\r?\n/g, '\r\n')
const doc = parse(exampleMdOriginalWindows)
assert.deepStrictEqual(exampleMdExpectedHunks, doc.dataHunks)
assert.deepStrictEqual(toString(doc), exampleMdOriginal)
})
})
7 changes: 6 additions & 1 deletion test/test-patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe('Patch', () => {
it('can patch example.md', async () => {
const exampleMd = new URL('../example.md', import.meta.url)
const exampleMdOriginal = await fs.promises.readFile(exampleMd, 'utf8')
const isWindows = exampleMdOriginal.includes('\r\n')
const patchedMd = new URL('./patch_expected.md', import.meta.url)
// extracted from patch_test.go
const patchedMdContents = await fs.promises.readFile(patchedMd, 'utf8')
Expand All @@ -20,6 +21,10 @@ describe('Patch', () => {
{ name: 'this-one-is-new', blockTag: 'json', body: '{"hayo": "new data!"}' },
{ name: 'so-is-this', blockTag: 'json', body: '{"appending": "is fun"}' }
])
assert.strictEqual(newDoc.original, patchedMdContents)
if (isWindows) {
assert.strictEqual(newDoc.original.replace(/\n/g, '\r\n'), patchedMdContents)
} else {
assert.strictEqual(newDoc.original, patchedMdContents)
}
})
})
2 changes: 1 addition & 1 deletion types/parse.d.ts.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.