-
-
Notifications
You must be signed in to change notification settings - Fork 817
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
feat: Add a way to escape { and }in exec templates. #1318
Conversation
9337df0
to
7aca019
Compare
doc/fd.1
Outdated
If you need to include the literal text of one of the placeholders, you can use "{{}" to | ||
escape the first "{". For example "{{}}" expands to "{}", and "{{}{{}}}" expands to "{{}". |
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.
@tmccombs Thank you very much for working on this and for even providing two implementations.
If I'm being really honest, this paragraph sounds incredible confusing at first. It does make sense and is logically consistent, but it's.. certainly not easy to grasp. This is a quite complex escaping rule. And one that I haven't seen in use anywhere else.
Given that you now implemented this with a custom tokenizer, is there still no way to implement Rusts {{
escaping or something like \{
?
What does parallel
do (we borrowed the --exec
syntax from them)?
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.
oops. This branch actually does implement using {{
and }}
for escapes. I just forgot to update the help and documentation to match 🤦.
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.
What does
parallel
do (we borrowed the--exec
syntax from them)?
I don't think you can escape braces in GNU parallel, but you can change the {}
to a different string: https://stackoverflow.com/questions/42532693/
ba9eacb
to
27d565c
Compare
27d565c
to
231aea2
Compare
src/cli.rs
Outdated
@@ -829,7 +831,8 @@ impl clap::Args for Exec { | |||
'{/}': basename\n \ | |||
'{//}': parent directory\n \ | |||
'{.}': path without file extension\n \ | |||
'{/.}': basename without file extension\n\n\ | |||
'{/.}': basename without file extension\n \ | |||
'{{}': literal '{' (for escaping)\n\n\ |
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.
is this still the old syntax? (comparing it with the above)
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.
Yeah, oops. Thanks for fixing that.
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.
Thank you very much.
Just one minor comment.
f2cd809
to
b881ad8
Compare
fixes #1303
Alternative to #1314