oconnor663 / duct.rs

a Rust library for running child processes
MIT License
795 stars 34 forks source link

Improve ergonomics of passing certain args conditionally #88

Open Boscop opened 3 years ago

Boscop commented 3 years ago

Currently the cmd!() macro only works for a fixed number of args. In real-world scenarios, often some args have to be passed only conditionally, and then the cmd!() macro can't be used. Leading to hard to read code where a let mut args: Vec<String> is constructed and conditionally pushed to. And then it makes the pushing of static arg strs unnecessarily verbose: args.push("-foo".to_string()); This becomes even less ergonomic when dealing with paths such that the whole Vec has to now be OsString.

The ergonomics of passing some args conditionally to cmd!() could be greatly improved by allowing cmd!() to take Option<T> where T: Into<OsString>! So that one can write: cmd!("prog", "-foo", cond.then_some("-bar"), "-baz")

After adding this, it also makes sense to add multiple args whose existence depends on the same condition. This requires allowing passing something like T: Iterator<Item = U> where U: Into<OsString> or simply Vec<U>/&[U]. So that one can write: cmd!("prog", "-foo", cond.then_some(&["-bar", "-baz"]))

With these small changes, the ergonomics of duct would be greatly improved :)


Notes on implementation: The bound of the cmd function would be changed to U::Item: Into<Option<OsString>>, and in it's body, it would then be:

argv_vec.extend(args.into_iter().filter_map(|opt_arg| opt_arg).map(Into::<OsString>::into));

But: We don't want to require all unconditional args to the cmd!() to be wrapped in Some. Although in the body of the macro, we can only do one .into() call, since we don't know whether the arg type is T: Option<OsString> or T: OsString. So solve this problem, it could help to introduce a new trait CmdArg (which is essentially acting like Into<Option<OsString>> but allowing a conversion from T: Into<OsString> to Option<OsString> in one function call (as well as from Option<OsString>) and then changing the trait bound of the cmd function to take U::Item: CmdArg.

oconnor663 commented 3 years ago

Interesting idea. It sounds like what we really want is one of two things:

Is it possible to implement something like CmdArg to abstract over this in stable Rust? Do we need to wait for specialization to land?

Boscop commented 3 years ago

@oconnor663 It seems it's not possible to due orphan rules to impl a trait for both cases, like:

trait CmdArg {}
impl<T: Into<OsString>> CmdArg for T {}
impl<T: Into<OsString>, I: IntoIterator<Item = T>> CmdArg for I {}

error[E0119]: conflicting implementations of trait CmdArg

So I suggest requiring the user to specify for each arg which case it is, with a small syntax addition, e.g. args that are iterators are prepended with .., like:

cmd!("prog", "-foo", ..cond.then_some(&["-bar", "-baz"]))