npm / rfcs

Public change requests/proposals & ideation
Other
726 stars 238 forks source link

[RRFC] Allow `npm completion` under MSYS2 #771

Open 500-internal-server-error opened 4 months ago

500-internal-server-error commented 4 months ago

Motivation ("The Why")

npm completion has worked under Git for Windows since 2015. Digging around a bit leads me to believe that back when the decision to explicitly not support Cygwin was made, it was due to Cygwin behavior with paths. However, Git for Windows is based on MSYS2, which has automatic path conversion to avoid Win32 programs seeing Unix paths. Cygwin/MSYS programs generally handle Win32 paths fine, but Win32 programs generally need the Unix paths converted before being passed to them. If my research was correct and the reason Cygwin was explicitly not supported was due to path problems, that reason is now gone with MSYS2. Thus, perhaps support for upstream MSYS2 can be considered (Cygwin still doesn't do automatic path conversions AFAICT, users still have to do that manually when passing paths between Cygwin and Win32 programs).

Example

username@hostname UCRT64 ~
$ npm completion >> ~/.zshrc

username@hostname UCRT64 ~
$

How

Current Behaviour

Currently, the Windows check only checks for Git for Windows:

const isWindowsShell = (process.platform === 'win32') &&
  !/^MINGW(32|64)$/.test(process.env.MSYSTEM) && process.env.TERM !== 'cygwin'

exports.isWindowsShell = isWindowsShell

Desired Behaviour

Given that the completion works under Git for Windows, it should also work against its direct upstream, so check for its environments as well:

diff --color -Naur a/lib/utils/is-windows.js b/lib/utils/is-windows.js
--- a/lib/utils/is-windows.js   2024-05-12 10:11:24.285665400 +0000
+++ b/lib/utils/is-windows.js   2024-05-12 10:11:36.695333600 +0000
@@ -1,4 +1,4 @@
 const isWindowsShell = (process.platform === 'win32') &&
-  !/^MINGW(32|64)$/.test(process.env.MSYSTEM) && process.env.TERM !== 'cygwin'
+  !/^((MSYS)|(MINGW|UCRT|CLANG|CLANGARM)(32|64))$/.test(process.env.MSYSTEM) && process.env.TERM !== 'cygwin'

 exports.isWindowsShell = isWindowsShell

As per an old comment:

won't merge any patch that's intended to solely change the behavior of npm for Cygwin.

I believe this change isn't a change for Cygwin in particular, its merely expanding an existing support a bit. In terms of path handling, there is less difference between Git for Windows and its upstream MSYS2 than there is between MSYS2 and its upstream Cygwin.

As a side note, the current support for Git for Windows does mean that MSYS2 users can already use the completion, but only if they're running the MINGW environment, which uses the MSVCRT runtime. Recently they've changed the primary suggested environment from MINGW64 to UCRT64, which uses the newer UCRT runtime. An overall harmless change, but it does mean that the completion no longer works.

References