assert_cli icon indicating copy to clipboard operation
assert_cli copied to clipboard

Deprecate macro in favor of clever `Assert::command(&str)`?

Open killercup opened this issue 8 years ago • 3 comments

What do you think about removing the macro and adding <T: ToCommandWithArgs> Assert::command(T) where ToCommandWithArgs is a trait implemented for &[u8] as well as &str? This way, you could run Assert::command(r#"cargo run --bin whatever -- --input="Lorem ipsum" -f"#).

In the &str case we'd parse it in a clever way to split the string by whitespace while preserving quoted blocks. I'm thinking of some split_args that fulfills

#[test]
fn test_arg_split() {
    assert_eq(split_args("echo 42"), vec!["echo", "42"]);

    assert_eq(split_args(r#"echo "42""#), vec!["echo", "\"42\""]);

    assert_eq(
        split_args(r#"cargo run --bin whatever -- --input="Lorem ipsum" -f"#),
        vec!["cargo", "run", "--bin", "whatever",  "--", "--input=\"Lorem ipsum\"", "-f"]
    );
}

(Copypasta from https://github.com/killercup/assert_cli/issues/22#issuecomment-288555281)

killercup avatar Mar 22 '17 23:03 killercup

tl;dr I suggest we keep Assert::command as-is and remove the macro.

The primary use case I see assert_cli being used for is to test someone's crate. At that point they'll be wanting to use main_binary or specify it some other means. This means there is low value for Assert::command accepting a single string. To support this, I just looked through all of the dependent crates on crates.io and none of them use the macro which is the current form of "single string" construction.

The main value I could see with having a macro of some form is to accept variable arguments that can all act as AsRef<str>. We'd then also want versions of the macro for various helpers (main_binary, etc). This is a separate concern and can be safely added to the API later without impact if it seems justified.

If a use case comes along for defining the command in a single string, I recommend we handle it then.

epage avatar Sep 27 '17 02:09 epage

I'm fine with removing the macro.

I'd like to have a way to specify a bunch of arguments at once without typing tons of quotes. What do you think about that?

(I think I opened a PR in spring to split a string with white space in a shell-like way. I'm also sure it didn't work but was the reason I wrote a post on writing a function that can take &str and &[&str]. Maybe I'm totally mistaken, I'll have to check once I'm in the office.)

Ed Page [email protected] schrieb am Mi. 27. Sep. 2017 um 04:59:

tl;dr I suggest we keep Assert::command as-is and remove the macro.

The primary use case I see assert_cli being used for is to test someone's crate. At that point they'll be wanting to use main_binary or specify it some other means. This means there is low value for Assert::command accepting a single string. To support this, I just looked through all of the dependent crates on crates.io and none of them use the macro which is the current form of "single string" construction.

The main value I could see with having a macro of some form is to accept variable arguments that can all act as AsRef. We'd then also want versions of the macro for various helpers (main_binary, etc). This is a separate concern and can be safely added to the API later without impact if it seems justified.

If a use case comes along for defining the command in a single string, I recommend we handle it then.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/killercup/assert_cli/issues/25#issuecomment-332394909, or mute the thread https://github.com/notifications/unsubscribe-auth/AABOX4MgDDpBYQua-dSR2wOO5vVQxaB1ks5smboSgaJpZM4Ml82L .

killercup avatar Sep 27 '17 07:09 killercup

It was PR #29. I think I'd like to revisit that idea but use it for .args(r#"--foo --bar="lol" --yay"#) instead of the ::command()

killercup avatar Sep 27 '17 08:09 killercup