search

sfackler / shell-escape

Apache License 2.0
20 stars 6 forks source link

Add support for escaping `OsStr` #9

Open aalekhpatel07 opened 1 year ago

aalekhpatel07 commented 1 year ago

Hi, thanks for this project!

A recent PR in openssh-rust/openssh provides an implementation for escape(&OsStr) -> Cow<'_, OsStr> that I think could be a good fit here.

It was suggested in a code review that it is preferable to use shell_escape for all the escaping needs rather than rolling out a custom implementation. However, currenly shell_escape only works with Cow<str>. This PR is an attempt to extend the escaping to OsStrs as well.

iiuc #5 is a relevant issue that we may be able to resolve with this PR.

Let me know if you have any suggestions/comments about these changes and I'll be happy to improve them accordingly.

aalekhpatel07 commented 1 year ago

Thanks for the quick review. I've applied some suggested changes and finished the tests. lmk what you think.