Skip to content

Commit

Permalink
feat(terminal): detect windows shell
Browse files Browse the repository at this point in the history
  • Loading branch information
imhoffd committed Oct 12, 2018
1 parent f4807f4 commit dcc912d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 30 deletions.
38 changes: 13 additions & 25 deletions packages/@ionic/cli-framework/src/utils/__tests__/process.ts
Original file line number Diff line number Diff line change
@@ -1,67 +1,55 @@
import { createProcessEnv } from '../process';

describe('@ionic/cli-framework', () => {

describe('utils/process', () => {

describe('createProcessEnv', () => {

describe('linux', () => {

const originalPlatform = process.platform;
describe('non-windows', () => {

beforeAll(() => {
Object.defineProperty(process, 'platform', { value: 'linux' });
});
const mock_terminal = { TERMINAL_INFO: { windows: false } };
jest.resetModules();
jest.mock('../terminal', () => mock_terminal);
const proc = require('../process');

it('should be case sensitive', () => {
const o = createProcessEnv();
const o = proc.createProcessEnv();
o['asdf'] = 'val1';
o['ASDF'] = 'val2';
expect(o).toEqual({ 'asdf': 'val1', 'ASDF': 'val2' });
});

it('should use sources and be case sensitive', () => {
const o = createProcessEnv({ 'hello': 'you' }, { 'hello': 'world' });
const o = proc.createProcessEnv({ 'hello': 'you' }, { 'hello': 'world' });
o['asdf'] = 'val1';
o['ASDF'] = 'val2';
expect(o['hello']).toEqual('world');
expect(o).toEqual({ 'asdf': 'val1', 'ASDF': 'val2', 'hello': 'world' });
});

afterAll(() => {
Object.defineProperty(process, 'platform', { value: originalPlatform });
});

});

describe('windows', () => {

const originalPlatform = process.platform;

beforeAll(() => {
Object.defineProperty(process, 'platform', { value: 'win32' });
});
const mock_terminal = { TERMINAL_INFO: { windows: true } };
jest.resetModules();
jest.mock('../terminal', () => mock_terminal);
const proc = require('../process');

it('should be case insensitive', () => {
const o = createProcessEnv();
const o = proc.createProcessEnv();
o['asdf'] = 'val1';
o['ASDF'] = 'val2';
expect(o).toEqual({ 'asdf': 'val2' });
});

it('should use sources and be case insensitive', () => {
const o = createProcessEnv({ 'HELLO': 'YOU' }, { 'hello': 'world' });
const o = proc.createProcessEnv({ 'HELLO': 'YOU' }, { 'hello': 'world' });
o['asdf'] = 'val1';
o['ASDF'] = 'val2';
expect(o['hello']).toEqual('world');
expect(o).toEqual({ 'asdf': 'val2', 'hello': 'world' });
});

afterAll(() => {
Object.defineProperty(process, 'platform', { value: originalPlatform });
});

});

});
Expand Down
11 changes: 6 additions & 5 deletions packages/@ionic/cli-framework/src/utils/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as lodash from 'lodash';
import * as kill from 'tree-kill';

import { createCaseInsensitiveObject } from './object';
import { TERMINAL_INFO } from './terminal';

const debug = Debug('ionic:cli-framework:utils:process');

Expand All @@ -24,13 +25,13 @@ export function killProcessTree(pid: number, signal: string | number = 'SIGTERM'
/**
* Creates an alternative implementation of `process.env` object.
*
* On Windows, `process.env` is a magic object that offers case-insensitive
* environment variable access. On other platforms, case sensitivity matters.
* This method creates an empty "`process.env`" object type that works for all
* platforms.
* On a Windows shell, `process.env` is a magic object that offers
* case-insensitive environment variable access. On other platforms, case
* sensitivity matters. This method creates an empty "`process.env`" object
* type that works for all platforms.
*/
export function createProcessEnv(...sources: { [key: string]: string | undefined; }[]): { [key: string]: string; } {
return lodash.assign(process.platform === 'win32' ? createCaseInsensitiveObject() : {}, ...sources);
return lodash.assign(TERMINAL_INFO.windows ? createCaseInsensitiveObject() : {}, ...sources);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/@ionic/cli-framework/src/utils/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,24 @@ if (CI_ENVIRONMENT_VARIABLES_DETECTED.length > 0) {
}

export interface TerminalInfo {
/**
* Whether the terminal is an interactive TTY or not.
*/
readonly tty: boolean;

/**
* Whether this is in CI or not.
*/
readonly ci: boolean;

/**
* Whether this is a Windows shell or not.
*/
readonly windows: boolean;
}

export const TERMINAL_INFO: TerminalInfo = Object.freeze({
tty: Boolean(process.stdin.isTTY && process.stdout.isTTY && process.stderr.isTTY),
ci: CI_ENVIRONMENT_VARIABLES_DETECTED.length > 0,
windows: process.platform === 'win32' && (process.env.MSYSTEM && /^MINGW(32|64)$/.test(process.env.MSYSTEM) || process.env.TERM === 'cygwin'),
});

3 comments on commit dcc912d

@micha-walter
Copy link

@micha-walter micha-walter commented on dcc912d Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

detecting the windows platform ist wrong. Please change line 36 in packages/@ionic/cli-framework/src/utils/terminal.ts from

windows: process.platform === 'win32' && (process.env.MSYSTEM && /^MINGW(32|64)$/.test(process.env.MSYSTEM) || process.env.TERM === 'cygwin'),

to

windows: process.platform === 'win32' || (process.env.MSYSTEM && /^MINGW(32|64)$/.test(process.env.MSYSTEM)) || process.env.TERM === 'cygwin',

@imhoffd
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @micha-walter! I was noticing that it was not detecting Windows on my Windows machine. This is why!

@ivanosire
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

detecting the windows platform ist wrong. Please change line 36 in packages/@ionic/cli-framework/src/utils/terminal.ts from

windows: process.platform === 'win32' && (process.env.MSYSTEM && /^MINGW(32|64)$/.test(process.env.MSYSTEM) || process.env.TERM === 'cygwin'),

to

windows: process.platform === 'win32' || (process.env.MSYSTEM && /^MINGW(32|64)$/.test(process.env.MSYSTEM)) || process.env.TERM === 'cygwin',

👍

Please sign in to comment.