npm / npm/rfcs

[RRFC] Allow `npm completion` under MSYS2

Open
#771 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
777
Forks
267
PR merge metrics
No merged PRs in 30d

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with lib/utils/is-windows.js and review the existing MSYS/MINGW environment check. Compare the listed MSYS2 environments with the current detection and verify npm completion behavior under the relevant shells. Done means the requested MSYS2 environments are supported without changing Cygwin behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
cli
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.