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

Remove unneeded quote escaping inside `` #26

Closed
wants to merge 1 commit into from

Conversation

laomaiweng
Copy link

Double quotes don't need to be escaped inside Bash command substitution (either ` ` or $()). However the current code works since ` ` behaves specially wrt. backslashes (see "Command Substitution" in man bash). (That is not the case with $(), as Sabayon/genkernel-next#49 shows -- genkernel-next upgraded the ` ` substitution to a $() one.)

Still, since the escapes aren't needed, and break $(), I suggest they be removed. Also, I upgraded the substitution to a $() one, which is IMO easier for humans to parse (in general, not necessarily in this particular case).

@robbat2
Copy link
Owner

robbat2 commented Mar 29, 2019

Please add signed-off-by to your commit and rebase, then I'll merge

Double quotes don't need to be escaped inside Bash command substitution.
Also switch to the more modern $(), easier for humans to parse (and which happens to be less forgiving about such extra backslash escapes).

Signed-off-by: Quentin Minster <[email protected]>
@laomaiweng
Copy link
Author

I have no idea if I'm doing things correctly with git but here you go, the branch seems rebased and the commit is signed-off-by. :)

@robbat2
Copy link
Owner

robbat2 commented May 16, 2022

Already fixed

@robbat2 robbat2 closed this May 16, 2022
@laomaiweng laomaiweng deleted the patch-1 branch June 29, 2022 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants