Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RRFC] Allow npm completion under MSYS2 #771

Open
500-internal-server-error opened this issue May 3, 2024 · 0 comments
Open

[RRFC] Allow npm completion under MSYS2 #771

500-internal-server-error opened this issue May 3, 2024 · 0 comments

Comments

@500-internal-server-error
Copy link

500-internal-server-error commented May 3, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant