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

Custom escapes don't work properly #5

Closed
haltman-at opened this issue Feb 1, 2023 · 4 comments
Closed

Custom escapes don't work properly #5

haltman-at opened this issue Feb 1, 2023 · 4 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@haltman-at
Copy link

Let's say I set the escape character to, say, "H" instead of backslash.

If I do parse("abcH def", {}, { escape: "H"}), the result ought to be ["abc def"], since the space was escaped. Instead, it doesn't work, and the result is ["abc", "def"]. The escape character has been correctly "eaten", but the space didn't actually get escaped.

Meanwhile, if I do parse("abc\\ def", {}, { escape: "H" }), then, since the backslash isn't acting as an escape, the result ought to be ["abc\\", "def"]. Instead one gets ["abc\\ def"]. The backslash is correctly not eaten, since it isn't acting as an escape... and yet the space gets escaped anyway, even though it shouldn't be!

I was thinking I might submit a PR for this, but I'm having trouble figuring out what is causing this... looking at the code, it appears correct. Dunno if something went wrong with the release process or something. Or maybe I'm just misreading and the bug really is there in the code and I can't see it. I don't know; well, here's an issue, hopefully you can figure it out.

Thank you!

(Tested with shell-quote versions 1.7.4 and 1.8.0)

@ljharb
Copy link
Owner

ljharb commented Feb 1, 2023

That does seem like a bug - but the example in the readme apparently shows the behavior you describe https://github.com/ljharb/shell-quote#parse-with-custom-escape-character. Did this ever work properly? The original commit that added it (in v1.5.0) 4d400e7 does have a test case tho.

@haltman-at
Copy link
Author

The example in the readme doesn't seem to show either the correct behavior or the one I described? It sets the escape character to "^", but then doesn't use any carets or backslashes in the string getting parsed. So it just doesn't show how the escape character works!

@ljharb
Copy link
Owner

ljharb commented Feb 1, 2023

ha, true enough.

i can reproduce the bug easily but i can't figure out how to fix it yet.

@ljharb ljharb added bug Something isn't working help wanted Extra attention is needed labels Feb 1, 2023
@ljharb
Copy link
Owner

ljharb commented Feb 1, 2023

Looks like the BAREWORD regex hardcodes \s in the regex, which means space characters can't be escaped by the current implementation.

@ljharb ljharb closed this as completed in 7bcd90e Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants