glob-parent icon indicating copy to clipboard operation
glob-parent copied to clipboard

Incorrect identification of the static part of the pattern for the disk root on Windows

Open mrmlnc opened this issue 2 years ago • 4 comments

What were you expecting to happen?

expect(gp('C:/', { flipBackslashes: false })).toEqual('C:/');
expect(gp('C:/.', { flipBackslashes: false })).toEqual('C:/');
expect(gp('C:/*', { flipBackslashes: false })).toEqual('C:/');
expect(gp('C:/./*', { flipBackslashes: false })).toEqual('C:/.');
expect(gp('C://', { flipBackslashes: false })).toEqual('C:/');
expect(gp('C://*', { flipBackslashes: false })).toEqual('C:/');

What actually happened?

expect(gp('C:/', { flipBackslashes: false })).toEqual('C:'); // 🔴 C: instead of C:/
expect(gp('C:/.', { flipBackslashes: false })).toEqual('C:'); // 🔴 C: instead of C:/
expect(gp('C:/*', { flipBackslashes: false })).toEqual('C:'); // 🔴 C: instead of C:/
expect(gp('C:/./*', { flipBackslashes: false })).toEqual('C:/.'); // 🟢 
expect(gp('C://', { flipBackslashes: false })).toEqual('C:/'); // 🟢 
expect(gp('C://*', { flipBackslashes: false })).toEqual('C:/'); // 🟢 

Please give us a sample of your gulpfile

The examples above are tests for this repository.

Please provide the following information:

  • OS & version [e.g. MacOS Catalina 10.15.4]: Windows 11 PRO 22H2
  • node version (run node -v): v20.0.0
  • npm version (run npm -v): 9.6.4
  • gulp version (run gulp -v): nope

Additional information

The current result is not correct because its use leads to incorrect results in standard Node methods.like path.* or fs.*:

CWD: D:\\OpenSource\\glob-parent

const path = require('path');

path.win32.resolve('D:'); // CWD
path.win32.resolve('D:/'); // D:\\

const fs = require('fs');

fs.readdirSync('D:'); // list CWD
fs.readdirSync('D:/'); // list D:\\

mrmlnc avatar Apr 19 '23 08:04 mrmlnc

Unfortunately, I don't know a better fix than the next one:

// https://github.com/gulpjs/glob-parent/blob/3a14ff5125d9fa9614dc214532327711fa6bbd93/index.js#L32

// remove path parts that are globby
do {
  if (isWin32 && !str.includes('\\')) {
    str = pathWindowsDirname(str); // path.win32.dirname
  } else {
    str = pathPosixDirname(str); // path.unix.dirname
  }
} while (isGlobby(str));

It is important for us to understand that C: is a drive on Windows.

The proposed fix also adds support for UNC paths like //?/C:/*.

mrmlnc avatar Apr 19 '23 08:04 mrmlnc

This seems like a good change. I'll look into making it soon (been very busy recently).

phated avatar Apr 22 '23 22:04 phated

@sttk do you have time to look into this?

phated avatar Jun 26 '23 00:06 phated

@mrmlnc @phated I've created the PR #64 to fix this issue. Please review it.

sttk avatar Jul 01 '23 09:07 sttk