-
Notifications
You must be signed in to change notification settings - Fork 343
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
Allow \0 escape for nulls #2251
Conversation
dcbb54e
to
dd1342b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly (er... to me), the following doesn't seem to print any null characters:
$ jj log -T $(printf '"a\0b"')
In fish
, I also get:
fish> jj log -T '"'a \0 b'"'
Error: Failed to parse template: --> 1:3
|
1 | "a
| ^---
|
= expected escape or raw_literal
Meanwhile, this does work.
$ jj log -T $(printf '"a\1b"')
So, it makes sense to add "\0" and not "\1".
Update: This is a shell issue. I managed to get the templater to parse a raw null char by adding the following to my TOML config. I don't think this possibility does much to subvert the point that entering a null byte without this PR is a little difficult.
[template-aliases]
tmp_null = "\"a\u0000\u0001b\""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you like, you can update test_string_literal()
in template_parser.rs. I think it's also good to have e2e test since null character is a bit special, so I wouldn't remove all of the CLI tests.
I believe it's actually impossible to pass a real null byte as a parameter: parameters are null-terminated strings all the way down to the exec syscalls. Presumably, bash is stripping out the null, and fish is probably actually setting the parameter exactly as the string you specified, and so ends up terminating the parameter early at the exec call. |
This allows safely getting e.g. multiple descriptions, and knowing where the boundaries are
dd1342b
to
800ffd4
Compare
Added to |
We are a little weird about which string escapes we support, and we don't support raw strings. I thought this might be worth documenting. Inspired by jj-vcs#2251
We are a little weird about which string escapes we support, and we don't support raw strings. I thought this might be worth documenting. Inspired by jj-vcs#2251
We are a little weird about which string escapes we support, and we don't support raw strings. I thought this might be worth documenting. Inspired by jj-vcs#2251
We are a little weird about which string escapes we support, and we don't support raw strings. I thought this might be worth documenting. Inspired by #2251
We are a little weird about which string escapes we support, and we don't support raw strings. I thought this might be worth documenting. Inspired by jj-vcs#2251
This allows safely getting e.g. multiple descriptions, and knowing where the boundaries are
Checklist
If applicable:
CHANGELOG.md