Skip to content

Commit

Permalink
Infer EOLs from existing file or use OS EOL (#1911)
Browse files Browse the repository at this point in the history
* When writing lockfile, yarnrc or manifest file, keep line endings as in existing file
* Implemented as proposed by @ahelmberger
* Resolves #1061
  • Loading branch information
iredchuk authored and bestander committed Nov 17, 2016
1 parent 4ce67f3 commit 05bf977
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 4 deletions.
28 changes: 28 additions & 0 deletions __tests__/commands/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import Config from '../../src/config.js';
import * as fs from '../../src/util/fs.js';
import assert from 'assert';
import semver from 'semver';
import {promisify} from '../../src/util/promise';
import fsNode from 'fs';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 90000;

Expand Down Expand Up @@ -576,3 +578,29 @@ test.concurrent('add should generate correct integrity file', (): Promise<void>
expect(allCorrect).toBe(true);
});
});

test.concurrent('add infers line endings from existing win32 manifest file', async (): Promise<void> => {
await runAdd({}, ['is-online'], 'add-infers-line-endings-from-existing-manifest-file',
async (config): Promise<void> => {
const lockfile = await promisify(fsNode.readFile)(path.join(config.cwd, 'package.json'), 'utf8');
assert(/\r\n/.test(lockfile));
assert(!/[^\r]\n/.test(lockfile));
},
async (cwd): Promise<void> => {
const existingLockfile = '{ "dependencies": {} }\r\n';
await promisify(fsNode.writeFile)(path.join(cwd, 'package.json'), existingLockfile, 'utf8');
});
});

test.concurrent('add infers line endings from existing unix manifest file', async (): Promise<void> => {
await runAdd({}, ['is-online'], 'add-infers-line-endings-from-existing-manifest-file',
async (config): Promise<void> => {
const lockfile = await promisify(fsNode.readFile)(path.join(config.cwd, 'package.json'), 'utf8');
assert(/[^\r]\n/.test(lockfile));
assert(!/\r\n/.test(lockfile));
},
async (cwd): Promise<void> => {
const existingLockfile = '{ "dependencies": {} }\n';
await promisify(fsNode.writeFile)(path.join(cwd, 'package.json'), existingLockfile, 'utf8');
});
});
37 changes: 37 additions & 0 deletions __tests__/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import * as fs from '../../src/util/fs.js';
import assert from 'assert';
import semver from 'semver';
import {getPackageVersion, explodeLockfile, runInstall, createLockfile} from './_install.js';
import {promisify} from '../../src/util/promise';
import fsNode from 'fs';
import os from 'os';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 90000;

Expand Down Expand Up @@ -692,3 +695,37 @@ test.concurrent('lockfile should be created when missing even if integrity match
expect(await fs.exists(path.join(config.cwd, 'yarn.lock')));
});
});

test.concurrent('install infers line endings from existing win32 lockfile', async (): Promise<void> => {
await runInstall({}, 'install-infers-line-endings-from-existing-lockfile',
async (config): Promise<void> => {
const lockfile = await promisify(fsNode.readFile)(path.join(config.cwd, 'yarn.lock'), 'utf8');
assert(/\r\n/.test(lockfile));
assert(!/[^\r]\n/.test(lockfile));
},
async (cwd): Promise<void> => {
const existingLockfile = '# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\r\n';
await promisify(fsNode.writeFile)(path.join(cwd, 'yarn.lock'), existingLockfile, 'utf8');
});
});

test.concurrent('install infers line endings from existing unix lockfile', async (): Promise<void> => {
await runInstall({}, 'install-infers-line-endings-from-existing-lockfile',
async (config): Promise<void> => {
const lockfile = await promisify(fsNode.readFile)(path.join(config.cwd, 'yarn.lock'), 'utf8');
assert(/[^\r]\n/.test(lockfile));
assert(!/\r\n/.test(lockfile));
},
async (cwd): Promise<void> => {
const existingLockfile = '# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n';
await promisify(fsNode.writeFile)(path.join(cwd, 'yarn.lock'), existingLockfile, 'utf8');
});
});

test.concurrent('install uses OS line endings when lockfile doesn\'t exist', async (): Promise<void> => {
await runInstall({}, 'install-infers-line-endings-from-existing-lockfile',
async (config): Promise<void> => {
const lockfile = await promisify(fsNode.readFile)(path.join(config.cwd, 'yarn.lock'), 'utf8');
assert(lockfile.indexOf(os.EOL) >= 0);
});
});
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"left-pad": "1.1.1"
}
}
2 changes: 1 addition & 1 deletion src/cli/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ export class Install {
const loc = path.join(this.config.cwd, constants.LOCKFILE_FILENAME);

// write lockfile
await fs.writeFile(loc, lockSource);
await fs.writeFilePreservingEol(loc, lockSource);

this._logSuccessSaveLockfile();
}
Expand Down
4 changes: 2 additions & 2 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ export default class Config {
return file;
}
}

return null;
});
}
Expand Down Expand Up @@ -484,7 +484,7 @@ export default class Config {
}
}

await fs.writeFile(loc, JSON.stringify(object, null, indent || constants.DEFAULT_INDENT) + '\n');
await fs.writeFilePreservingEol(loc, JSON.stringify(object, null, indent || constants.DEFAULT_INDENT) + '\n');
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/registries/yarn-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,6 @@ export default class YarnRegistry extends NpmRegistry {
this.homeConfig[key] = config[key];
}

await fs.writeFile(this.homeConfigLoc, `${stringify(this.homeConfig)}\n`);
await fs.writeFilePreservingEol(this.homeConfigLoc, `${stringify(this.homeConfig)}\n`);
}
}
30 changes: 30 additions & 0 deletions src/util/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import map from './map.js';

const path = require('path');
const fs = require('fs');
const os = require('os');

export const lockQueue = new BlockingQueue('fs lock');

Expand Down Expand Up @@ -454,3 +455,32 @@ export async function getFileSizeOnDisk(loc: string): Promise<number> {
export function normalizeOS(body: string): string {
return body.replace(/\r\n/g, '\n');
}

const cr = new Buffer('\r', 'utf8')[0];
const lf = new Buffer('\n', 'utf8')[0];
async function getEolFromFile(path: string) : Promise<string | void> {
if (!(await exists(path))) {
return undefined;
}
const buffer = await readFileBuffer(path);
for (let i = 0; i < buffer.length; ++i) {
if (buffer[i] === cr) {
return '\r\n';
}
if (buffer[i] === lf) {
return '\n';
}
}
return undefined;
}
export async function writeFilePreservingEol(path: string, data: string) : Promise<void> {
const eol = (await getEolFromFile(path)) || os.EOL;
if (eol !== '\n') {
data = data.replace(/\n/g, eol);
}
await promisify(fs.writeFile)(path, data);
}

0 comments on commit 05bf977

Please sign in to comment.