micromatch / braces

Faster brace expansion for node.js. Besides being faster, braces is not subject to DoS attacks like minimatch, is more accurate, and has more complete support for Bash 4.3.
https://github.com/jonschlinkert
MIT License
207 stars 47 forks source link

documentation: "keepEscaping" is missing #26

Closed vemoo closed 5 years ago

vemoo commented 5 years ago

I'm not sure if it's a braces or micromatch issue, but

braces("C:/Program Files \\(x86\\)")

returns

[ "C:/Program Files (x86)" ]

losing the escape characters, making

micromatch.parse("C:/Program Files \\(x86\\)")

return the tokens

[
    {
        "type": "bos",
        "value": "",
        "output": ""
    },
    {
        "type": "text",
        "value": "C:"
    },
    {
        "type": "slash",
        "value": "/",
        "output": "\\/"
    },
    {
        "type": "text",
        "value": "Program Files "
    },
    {
        "type": "paren",
        "value": "("
    },
    {
        "type": "text",
        "value": "x86"
    },
    {
        "type": "paren",
        "value": ")",
        "output": ")"
    }
]

vs what I expected and what

picomatch.parse("C:/Program Files \\(x86\\)")

returns

[
    {
        "type": "bos",
        "value": "",
        "output": ""
    },
    {
        "type": "text",
        "value": "C:"
    },
    {
        "type": "slash",
        "value": "/",
        "output": "\\/"
    },
    {
        "type": "text",
        "value": "Program Files \\(x86\\)"
    }
]

Here's some sample code: https://repl.it/repls/SharpStrangeChemistry

vemoo commented 5 years ago

I've realized theres options.unescape but it doesn't seem to do anything in this case.

jonschlinkert commented 5 years ago

That's not right, this is a bug, and it's inconsistent with how picomatch works. It should allow you to retain escaping. Thank you for reporting it.

jonschlinkert commented 5 years ago

ah, wait. The option is keepEscaping. I forgot that I used a different option name to allow you to escape one or the other, or both. If both picomatch and braces had the same option name (unescape) for this specifically, you wouldn't be able to control escaping.

Try this:

console.log(braces("C:/Program Files \\(x86\\)", { keepEscaping: true }));
vemoo commented 5 years ago

Yes, that works. Then the documentation should be updated and it should be added to the documentation of micromatch.

jonschlinkert commented 5 years ago

Then the documentation should be updated and it should be added to the documentation of micromatch.

Agreed! Thanks for reporting this. Want to do a PR?