denoland / std

The Deno Standard Library
https://jsr.io/@std
MIT License
3.01k stars 602 forks source link

isGlob and globToRegExp do not support windows paths #5434

Open andykais opened 2 months ago

andykais commented 2 months ago

Describe the bug

Steps to Reproduce

import * as path from 'jsr:@std/path@1.0.0'

let globpath = 'D:\\a\\forager\\test\\resources\\*.jpg'
path.isGlob(globpath)
// false
path.globToRegExp(globpath)
// /^D:aforagertestresources\*\.jpg\/*$/

Expected behavior I would expect to see globpath above return true for a filepath that contains * even on windows. I would also expect that globToRegExp would create a valid path delineated regex. See how these functions behave with unix paths below:

import * as path from 'jsr:@std/path@1.0.0'

let globpath = '/forager/test/resources/*.jpg'
path.isGlob(globpath)
// true
path.globToRegExp(globpath)
// /^\/+forager\/+test\/+resources\/+[^/]*\.jpg\/*$/

My best guess is that this is because globs are mainly used in unix systems. I am building a cross-platform app though that needs some kind of glob support for windows paths. For now I can fork these methods to be more cross-platform, but I think this is probably a good improvement to these utilities.

Environment

luk3skyw4lker commented 2 months ago

I tried the case of isGlob with a windows path and the bug happened, so it's real.

iuioiua commented 2 months ago

@std/path functions change how they process paths based on the current machine's platform. You can change this behavior by instead importing from @std/path/<platform> for a specific platform. I.e., the following code give the following results.

import * as path from "@std/path/windows";

const globpath = "D:\\a\\forager\\test\\resources\\*.jpg";
console.log(path.isGlob(globpath));
// false;
console.log(path.globToRegExp(globpath));
// /^D:(?:\\|\/)+a(?:\\|\/)+forager(?:\\|\/)+test(?:\\|\/)+resources(?:\\|\/)+[^\\/]*\.jpg(?:\\|\/)*$/

Either way, from a glance, the first call is still incorrect, but the second call seems closer to the expected result. It looks like isGlob() might need some work...

kt3k commented 2 months ago

I'm not sure this is a bug or not.

It looks like the current isGlob seems assuming windows path separator is given as four backslashes:

\\\\

(I guess this is probably because \ is considered as escape sequence in glob.)

This behavior seems aligned with is-glob npm package.

import * as path from 'jsr:@std/path@1.0.0'
import isGlob from "npm:is-glob";

let globpath0 = 'D:\\a\\forager\\test\\resources\\*.jpg'
let globpath1= 'D:\\\\a\\\\forager\\\\test\\\\resources\\\\*.jpg'

console.log(path.isGlob(globpath0))
console.log(isGlob(globpath0));

console.log(path.isGlob(globpath1))
console.log(isGlob(globpath1));

this prints:

false
false
true
true
iuioiua commented 2 months ago

Oh! That makes sense.

andykais commented 1 week ago

@std/path functions change how they process paths based on the current machine's platform. You can change this behavior by instead importing from @std/path/ for a specific platform. I.e., the following code give the following results.

hmm so @iuioiua if my paths are real windows paths, will a unix path regex actually work for me? I assume parsing a glob with path.globToRegExp from the unix platform will produce a regex that works with unix paths, not windows paths