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

Improve Windows support for escaped glob syntax #12718

Closed

Conversation

RobinMalfait
Copy link
Contributor

This PR is a continuation of #12715 to apply the same improvements for Windows.

On Windows the path separator is \\ and \\ is also used for escaping the [, ], ( and ) characters. This will result in \\\\[ for example. In this case it means that the first \\ is the path separator and the second \\ is the escape for the [ or ( characters.

Additionally, we also ensure that we only escape [, ] and (, ) when the balanced pairs are both not escaped yet. Right now it was going over the individual characters and escaping them accordingly.

This is not enough, because in case of this path:

let path = ".\\src\\**\\[foo]\\bar.html"

The first \\[ looks like it is escaped, but on Windows this is just the path separator. The foo] part is not escaped therefor we should escape the whole thing. As a first step, this will result in:

let path = ".\\src\\**\\\\[foo\\]\\bar.html"

Now we do have the 4 backslashes \\\\, then in the normalization step, we normalize \\ to /. If we do this as is, then it would look like this, which is also incorrect:

let path = "./src/**/[foo/]/bar.html"

... it's incorrect because we have /[foo/] and we expected /\\[foo\\] instead.

If we apply the logic I mentioned earlier where the first \\ is the path separator and the second \\ is the escape, then we can convert:

let path = ".\\src\\**\\\\[foo\\]\\bar.html"

... to this first:

let path = ".\\src\\**/\\[foo\\]\\bar.html"

Then we can apply the rest of the normalization, which looks like this:

let path = "./src/**/\\[foo\\]/bar.html"

... and now the [foo] part is properly escaped as \\[foo\\].

@RobinMalfait RobinMalfait force-pushed the fix/issue-12708-windows branch 2 times, most recently from 6225eef to f2f6cfe Compare January 5, 2024 23:04
@mominshaikhdevs
Copy link

please merge this PR soon as its absolutely essential.

@thecrypticace
Copy link
Contributor

It is unfortunately not finished as it doesn't work 100% yet on Windows after I did some testing on Friday.

@RobinMalfait RobinMalfait changed the base branch from master to archive/master-2024-02-23 March 4, 2024 21:49
@RobinMalfait
Copy link
Contributor Author

Going to close this PR for now because the original issue that was reported was for macOS / Linux. This is probably still a good fix to do, but the current implementation is not ideal just because an escaped \\ also happens to be a path separator in Windows which means that this is ambiguous.

The real fix should be to split the base path and the actual glob so that the ambiguity goes away. Because then the path can contain \\ as the separator and the glob itself containing \\ will be a guaranteed escape value.

@RobinMalfait RobinMalfait deleted the fix/issue-12708-windows branch March 21, 2024 13:48
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

Successfully merging this pull request may close these issues.

None yet

3 participants