Skip to content

Commit

Permalink
chore(utils): replace last expandPath with resolveWithHome (yarnp…
Browse files Browse the repository at this point in the history
…kg#4605)

**Summary**

Looking at two solutions introduced in yarnpkg#3393 and yarnpkg#3756, the first one doesn't support win32, while the second does, sticking with the second one more beneficial and supports a wider range of OS.

Removed the stuff introduced in yarnpkg#3393 keeping only yarnpkg#3756.

yarnpkg#3756 also introduced config file normalization, so probably second argument to getOption is obsolete, will discover that and submit another PR if that's the case.

**Test plan**

Modified tests appropriately.
  • Loading branch information
viatsko authored and joaolucasl committed Oct 27, 2017
1 parent 82ceb3e commit 530280f
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 56 deletions.
2 changes: 1 addition & 1 deletion __tests__/fixtures/global/add-with-prefix-env/.yarnrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
prefix ""
prefix false
40 changes: 1 addition & 39 deletions __tests__/util/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,45 +12,7 @@ jest.mock('path', () => {
return path;
});

import {expandPath, resolveWithHome} from '../../src/util/path.js';

describe('expandPath', () => {
const realPlatform = process.platform;

describe('!win32', () => {
beforeAll(() => {
process.platform = 'notWin32';
});

afterAll(() => {
process.platform = realPlatform;
});

test('Paths get expanded', () => {
expect(expandPath('~/bar/baz/q')).toEqual('/home/foo/bar/baz/q');
expect(expandPath(' ~/bar/baz')).toEqual('/home/foo/bar/baz');
expect(expandPath('./~/bar/baz')).toEqual('./~/bar/baz');
expect(expandPath('~/~/bar/baz')).toEqual('/home/foo/~/bar/baz');
});
});

describe('win32', () => {
beforeAll(() => {
process.platform = 'win32';
});

afterAll(() => {
process.platform = realPlatform;
});

test('Paths never get expanded', () => {
expect(expandPath('~/bar/baz/q')).toEqual('~/bar/baz/q');
expect(expandPath(' ~/bar/baz')).toEqual(' ~/bar/baz');
expect(expandPath('./~/bar/baz')).toEqual('./~/bar/baz');
expect(expandPath('~/~/bar/baz')).toEqual('~/~/bar/baz');
});
});
});
import {resolveWithHome} from '../../src/util/path.js';

describe('resolveWithHome', () => {
const realPlatform = process.platform;
Expand Down
9 changes: 5 additions & 4 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {Reporter} from './reporters/index.js';
import type {Manifest, PackageRemote, WorkspacesManifestMap} from './types.js';
import type PackageReference from './package-reference.js';
import {execFromManifest} from './util/execute-lifecycle-script.js';
import {expandPath} from './util/path.js';
import {resolveWithHome} from './util/path.js';
import normalizeManifest from './util/normalize-manifest/index.js';
import {MessageError} from './errors.js';
import * as fs from './util/fs.js';
Expand Down Expand Up @@ -192,10 +192,11 @@ export default class Config {
* Get a config option from our yarn config.
*/

getOption(key: string, expand: boolean = false): mixed {
getOption(key: string, resolve: boolean = false): mixed {
const value = this.registries.yarn.getOption(key);
if (expand && typeof value === 'string') {
return expandPath(value);

if (resolve && typeof value === 'string') {
return resolveWithHome(value);
}

return value;
Expand Down
6 changes: 3 additions & 3 deletions src/registries/npm-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ function getGlobalPrefix(): string {
}
}

const PATH_CONFIG_OPTIONS = ['cache', 'cafile', 'prefix', 'userconfig'];
const PATH_CONFIG_OPTIONS = new Set(['cache', 'cafile', 'prefix', 'userconfig']);

function isPathConfigOption(key: string): boolean {
return PATH_CONFIG_OPTIONS.indexOf(key) >= 0;
return PATH_CONFIG_OPTIONS.has(key);
}

function normalizePath(val: mixed): ?string {
Expand All @@ -65,7 +65,7 @@ function normalizePath(val: mixed): ?string {
}

if (typeof val !== 'string') {
val = '' + (val: any);
val = String(val);
}

return resolveWithHome(val);
Expand Down
10 changes: 1 addition & 9 deletions src/util/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,9 @@ export function getPosixPath(path: string): string {
return path.replace(/\\/g, '/');
}

export function expandPath(path: string): string {
if (process.platform !== 'win32') {
path = path.replace(/^\s*~(?=$|\/|\\)/, userHome);
}

return path;
}

export function resolveWithHome(path: string): string {
const homePattern = process.platform === 'win32' ? /^~(\/|\\)/ : /^~\//;
if (path.match(homePattern)) {
if (homePattern.test(path)) {
return resolve(userHome, path.substr(2));
}

Expand Down

0 comments on commit 530280f

Please sign in to comment.