Conversation
Molter73
left a comment
Molter73
left a comment
There was a problem hiding this comment.
Couple small comments, overall looks good! Thanks for tackling this.
| def rust_style_quote(s): | ||
| if not s: | ||
| return "''" | ||
| if re.search(r'[^a-zA-Z0-9_./-]', s): | ||
| # Try to match the behavior of shlex.try_join() | ||
| if '\'' in s and not '"' in s: | ||
| return f'"{s}"' | ||
| escaped = s.replace("'", "\\'") | ||
| return f"'{escaped}'" | ||
| return s | ||
|
|
||
| def rust_style_join(args): | ||
| return ' '.join(rust_style_quote(arg) for arg in args) |
There was a problem hiding this comment.
Have you tried using Python's shlex and see if it behaves the same way as the Rust implementation? It would be a lot easier for tests.
There was a problem hiding this comment.
This is what I used initially, and their quoting strategy is vastly different, unfortunately.
There was a problem hiding this comment.
Well, "vastly" was maybe a bit too strong : the "safe" characters list differ, in particular rust considers '=' and ':' unsafe.
This behavior is not tunable AFAICT, so I resorted to mimic things on the testing side.
There was a problem hiding this comment.
Could you put a comment detailing this so the next person doesn't try to use Python's shlex?
| let args = | ||
| shlex::try_join(args.iter().map(|s| s.as_str())).unwrap_or_else(|_| String::new()); |
There was a problem hiding this comment.
Since the parsing of arguments received from the kernel is done by searching for null characters, I don't think it is possible for shlex::try_join to fail here, because we've already removed them all. If that's the case, this might be better:
| let args = | |
| shlex::try_join(args.iter().map(|s| s.as_str())).unwrap_or_else(|_| String::new()); | |
| let Ok(args) = shlex::try_join(args.iter().map(|s| s.as_str())) else { | |
| unreachable!(); | |
| }; |
Probably also want to update the comment to reflect this.
|
BTW, I'm working on improving the output of test errors because it is horrible at the moment: #296 |
2 participants
Description
Introducing proper quoting of arguments when serializing the command-line.
Checklist
Automated testing