-
Notifications
You must be signed in to change notification settings - Fork 232
Fix #28 -- escaping arguments as mentioned in TODO #2093
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
Conversation
lib/src/command_runner.dart
Outdated
// Escape the argument for users to copy-paste in bash. | ||
protectArgument(String x) => | ||
RegExp(r'^[a-zA-Z0-9-_]+$').stringMatch(x) == null | ||
? "'${x.replaceAll("'", "'\\''")}'" |
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.
You cannot escape a single quote inside a single quote:
https://www.gnu.org/software/bash/manual/html_node/Single-Quotes.html#Single-Quotes
You probably need to use double quotes and escape all of ‘$’, ‘`’, ‘\’
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.
Not really... Read carefully I escape single quote, by ending single quotation then having an escaped single quote and starting single quotation again :)
If there is no space between end and start of single quotation it doesn't become a new argument...
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.
Explain this in a comment ...
lib/src/command_runner.dart
Outdated
// Escape the argument for users to copy-paste in bash. | ||
protectArgument(String x) => | ||
RegExp(r'^[a-zA-Z0-9-_]+$').stringMatch(x) == null | ||
? "'${x.replaceAll("'", "'\\''")}'" |
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.
You could use r"'\''"
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.
LGTM
I just want to close: #28