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

Failing to parse "\\(" #1991

Closed
JesseSchobben-TomTom opened this issue Mar 26, 2024 · 5 comments
Closed

Failing to parse "\\(" #1991

JesseSchobben-TomTom opened this issue Mar 26, 2024 · 5 comments
Labels

Comments

@JesseSchobben-TomTom
Copy link

Describe the bug
yq 4.43.1 introduces string interpolation, but it doesn't play nice with escaped braces in a regex.

Version of yq: 4.43.1
Operating system: mac
Installed via: homebrew

Command
The following throws an error:

$ yq -n '"test(" | match("test\\(")'
Error: unclosed interpolation string \(

With a single backslash that failure would be expected, but for the above command I'd expect a successful match reporting "test(".

Expected behavior

Maybe we should follow the behavior of jq here:

$ jq -n '"test(" | match("test\(")'
jq: error: syntax error, unexpected $end, expecting QQSTRING_TEXT or QQSTRING_INTERP_START or QQSTRING_END (Unix shell quoting issues?) at <top-level>, line 1:
"test(" | match("test\(")                        
jq: 1 compile error

$ jq -n '"test(" | match("test\\(")'
{
  "offset": 0,
  "length": 5,
  "string": "test(",
  "captures": []
}

Additional context
As side info, with yq 4.42.1 the single-backslash variant works, but double-backslash fails, so we'll have an incompatibility either way:

$ yq -n '"test(" | match("test\(")'
string: test(
offset: 0
length: 5
captures: []

$ yq -n '"test(" | match("test\\(")'
Error: error parsing regexp: missing closing ): `test\\(`
@mbenson
Copy link
Contributor

mbenson commented Apr 2, 2024

I am also encountering issues with this feature, namely that I want to do function-like stuff by saving off eval strings as variables for later use, which means I want to be able to escape a \() construct for evaluation later. I hadn't realized the functionality was so new... is that right that it was just released in a point release? I have a patch that I think will address the issue, but it alters the behavior of one of the unit tests by interpreting \\ constructs in the "normal" way, i.e. by assuming the first backslash escapes the second. I'll send a PR for consideration after I test it a bit.

@mbenson
Copy link
Contributor

mbenson commented Apr 2, 2024

"point release" I see there is no .0 😄

@mbenson
Copy link
Contributor

mbenson commented Apr 2, 2024

#1997

@mbenson
Copy link
Contributor

mbenson commented Apr 15, 2024

@mikefarah merged #1997 over the weekend... I actually never tried your original example, @JesseSchobben-TomTom , but I just have and it seems to work fine.

@mikefarah
Copy link
Owner

Got @mbenson 's fix in 4.44.1 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants