fix: consolidate is-windows code

Author: wraithgar

lib/commands/completion.js

@@ -37,7 +37,7 @@ const nopt = require('nopt')
 const configNames = Object.keys(definitions)
 const shorthandNames = Object.keys(shorthands)
 const allConfs = configNames.concat(shorthandNames)
-const isWindowsShell = require('../utils/is-windows-shell.js')
+const { isWindowsShell } = require('../utils/is-windows.js')
 const fileExists = require('../utils/file-exists.js')
 
 const { promisify } = require('util')

lib/commands/run-script.js

@@ -5,7 +5,7 @@ const { isServerPackage } = runScript
 const rpj = require('read-package-json-fast')
 const log = require('../utils/log-shim.js')
 const didYouMean = require('../utils/did-you-mean.js')
-const isWindowsShell = require('../utils/is-windows-shell.js')
+const { isWindowsShell } = require('../utils/is-windows.js')
 
 const cmdList = [
   'publish',

lib/utils/config/definitions.js

@@ -7,7 +7,7 @@ const { version: npmVersion } = require('../../../package.json')
 const ciDetect = require('@npmcli/ci-detect')
 const ciName = ciDetect()
 const querystring = require('querystring')
-const isWindows = require('../is-windows.js')
+const { isWindows } = require('../is-windows.js')
 const { join } = require('path')
 
 // used by cafile flattening to flatOptions.ca

lib/utils/error-message.js

@@ -60,7 +60,7 @@ module.exports = (er, npm) => {
         npm.config.loaded &&
         er.dest.startsWith(npm.config.get('cache'))
 
-      const isWindows = require('./is-windows.js')
+      const { isWindows } = require('./is-windows.js')
 
       if (!isWindows && (isCachePath || isCacheDest)) {
         // user probably doesn't need this, but still add it to the debug log

lib/utils/is-windows-bash.js

@@ -1 +1,6 @@
-module.exports = process.platform === 'win32'
+const isWindows = process.platform === 'win32'
+const isWindowsShell = isWindows &&
+  !/^MINGW(32|64)$/.test(process.env.MSYSTEM) && process.env.TERM !== 'cygwin'
+
+exports.isWindows = isWindows
+exports.isWindowsShell = isWindowsShell

lib/utils/is-windows-shell.js

@@ -1,4 +1,5 @@
 // return the PATH array in a cross-platform way
+// TODO this is only used in a single place
 const PATH = process.env.PATH || process.env.Path || process.env.path
 const { delimiter } = require('path')
 module.exports = PATH.split(delimiter)

lib/utils/is-windows.js

@@ -17,7 +17,7 @@ const loadMockCompletion = async (t, o = {}) => {
   }
   const res = await _loadMockNpm(t, {
     mocks: {
-      '../../lib/utils/is-windows-shell.js': !!windows,
+      '../../lib/utils/is-windows.js': { isWindowsShell: !!windows },
       ...options.mocks,
     },
     ...options,

lib/utils/path.js

@@ -47,7 +47,6 @@ const output = []
 const logs = []
 const getExplore = (windows) => {
   const Explore = t.mock('../../../lib/commands/explore.js', {
-    '../../../lib/utils/is-windows.js': windows,
     path: require('path')[windows ? 'win32' : 'posix'],
     'read-package-json-fast': mockRPJ,
     '@npmcli/run-script': mockRunScript,

test/lib/commands/completion.js

@@ -62,7 +62,7 @@ const getRS = windows => {
       }
     ),
     'proc-log': log,
-    '../../../lib/utils/is-windows-shell.js': windows,
+    '../../../lib/utils/is-windows.js': { isWindowsShell: windows },
   })
   return new RunScript(npm)
 }
@@ -859,7 +859,7 @@ t.test('workspaces', t => {
         throw new Error('err')
       },
       'proc-log': log,
-      '../../../lib/utils/is-windows-shell.js': false,
+      '../../../lib/utils/is-windows.js': { isWindowsShell: false },
     })
     const runScript = new RunScript(npm)
 
@@ -877,7 +877,7 @@ t.test('workspaces', t => {
         RUN_SCRIPTS.push(opts)
       },
       'proc-log': log,
-      '../../../lib/utils/is-windows-shell.js': false,
+      '../../../lib/utils/is-windows.js': { isWindowsShell: false },
     })
     const runScript = new RunScript(npm)
 

test/lib/commands/explore.js

@@ -53,11 +53,11 @@ t.test('editor', t => {
   t.test('has neither EDITOR nor VISUAL, system specific', t => {
     mockGlobals(t, { 'process.env': { EDITOR: undefined, VISUAL: undefined } })
     const defsWin = t.mock(defpath, {
-      [isWin]: true,
+      [isWin]: { isWindows: true },
     })
     t.equal(defsWin.editor.default, 'notepad.exe')
     const defsNix = t.mock(defpath, {
-      [isWin]: false,
+      [isWin]: { isWindows: false },
     })
     t.equal(defsNix.editor.default, 'vi')
     t.end()
@@ -69,12 +69,12 @@ t.test('shell', t => {
   t.test('windows, env.ComSpec then cmd.exe', t => {
     mockGlobals(t, { 'process.env.ComSpec': 'command.com' })
     const defsComSpec = t.mock(defpath, {
-      [isWin]: true,
+      [isWin]: { isWindows: true },
     })
     t.equal(defsComSpec.shell.default, 'command.com')
     mockGlobals(t, { 'process.env.ComSpec': undefined })
     const defsNoComSpec = t.mock(defpath, {
-      [isWin]: true,
+      [isWin]: { isWindows: true },
     })
     t.equal(defsNoComSpec.shell.default, 'cmd')
     t.end()
@@ -83,12 +83,12 @@ t.test('shell', t => {
   t.test('nix, SHELL then sh', t => {
     mockGlobals(t, { 'process.env.SHELL': '/usr/local/bin/bash' })
     const defsShell = t.mock(defpath, {
-      [isWin]: false,
+      [isWin]: { isWindows: false },
     })
     t.equal(defsShell.shell.default, '/usr/local/bin/bash')
     mockGlobals(t, { 'process.env.SHELL': undefined })
     const defsNoShell = t.mock(defpath, {
-      [isWin]: false,
+      [isWin]: { isWindows: false },
     })
     t.equal(defsNoShell.shell.default, 'sh')
     t.end()
@@ -158,18 +158,18 @@ t.test('unicode allowed?', t => {
 t.test('cache', t => {
   mockGlobals(t, { 'process.env.LOCALAPPDATA': 'app/data/local' })
   const defsWinLocalAppData = t.mock(defpath, {
-    [isWin]: true,
+    [isWin]: { isWindows: true },
   })
   t.equal(defsWinLocalAppData.cache.default, 'app/data/local/npm-cache')
 
   mockGlobals(t, { 'process.env.LOCALAPPDATA': undefined })
   const defsWinNoLocalAppData = t.mock(defpath, {
-    [isWin]: true,
+    [isWin]: { isWindows: true },
   })
   t.equal(defsWinNoLocalAppData.cache.default, '~/npm-cache')
 
   const defsNix = t.mock(defpath, {
-    [isWin]: false,
+    [isWin]: { isWindows: false },
   })
   t.equal(defsNix.cache.default, '~/.npm')
 

test/lib/commands/run-script.js

@@ -1,8 +1,39 @@
 const t = require('tap')
-const actuallyWindows = process.platform === 'win32'
-t.equal(actuallyWindows, require('../../../lib/utils/is-windows.js'))
-Object.defineProperty(process, 'platform', {
-  value: actuallyWindows ? 'posix' : 'win32',
+
+const mockGlobals = require('../../fixtures/mock-globals')
+
+t.test('is not windows', async t => {
+  mockGlobals(t, { 'process.platform': 'posix' })
+  t.match({
+    isWindows: false,
+    isWindowsShell: false,
+  }, t.mock('../../../lib/utils/is-windows.js'))
+})
+
+t.test('is windows, shell', async t => {
+  mockGlobals(t, {
+    'process.platform': 'win32',
+    'process.env': {
+      MSYSTEM: 'notmingw',
+      TERM: 'notcygwin',
+    },
+  })
+  t.match({
+    isWindows: true,
+    isWindowsShell: true,
+  }, t.mock('../../../lib/utils/is-windows.js'))
+})
+
+t.test('is windows, not shell', async t => {
+  mockGlobals(t, {
+    'process.platform': 'win32',
+    'process.env': {
+      MSYSTEM: 'MINGW32',
+      TERM: 'cygwin',
+    },
+  })
+  t.match({
+    isWindows: true,
+    isWindowsShell: false,
+  }, t.mock('../../../lib/utils/is-windows.js'))
 })
-delete require.cache[require.resolve('../../../lib/utils/is-windows.js')]
-t.equal(!actuallyWindows, require('../../../lib/utils/is-windows.js'))

test/lib/utils/config/definitions.js

@@ -1,6 +1,7 @@
 const t = require('tap')
 const mod = '../../../lib/utils/path.js'
-const delim = require('../../../lib/utils/is-windows.js') ? ';' : ':'
+const { isWindows } = require('../../../lib/utils/is-windows.js')
+const delim = isWindows ? ';' : ':'
 Object.defineProperty(process, 'env', {
   value: {},
 })