allow_invalid_utf8(true) required only in debug build

[njaard]

Please complete the following tasks

• I have searched the discussions
• I have searched the existing issues

Rust Version

1.58.1

Clap Version

3.0.12

Minimal reproducible code

	use clap::Arg;
	let matches = clap::App::new("app")
		.arg(Arg::new("f")
			.takes_value(true)
		)
		.get_matches();
	matches.value_of_os("f");

This program fails like so:

thread 'main' panicked at 'Must use `Arg::allow_invalid_utf8` with `_os` lookups at `f`', src/main.rs:12:13

But only in a debug build.

Steps to reproduce the bug with the above code

Succeeds: cargo run --release -- blah
Fails: cargo run -- blah

Actual Behaviour

Is allow_invalid_utf8(true) actually required? i don't know

Expected Behaviour

It should just work.

Additional Context

Why is this option even required? It's a giant footgun.

If you really want to do that check, maybe change takes_value to takes_string and takes_path. But as it is, it serves no purpose.

Debug Output

No response

[epage]

Why is this option even required? It's a giant footgun.

Clap was more of a footgun before this existed

In v2, if you did value_of and the user passed an invalid utf-8 string, clap would panic. Your options were one of

• Pass AppSettings::StrictUtf8 which prevented invalid utf-8 when it was intended
• Always use value_of_os and do the conversion yourself
• Write your own validators and make sure your accesses matched your definitions with those validators

Now, StrictUtf8 is the default and you can opt-out with allow_invalid_utf8 on a per-argument basis.

The asserts serve a couple of purposes

• Help users know that value_of_os should be paired with allow_invalid_utf8 to be effective (if you have strict utf-8, value_of_os offers you nothing that value_of gives you, if you use allow invalid utf-8, value_of will panic on invalid characters
  • This eases the transition from clap2, eases general on-boarding, and lower maintenance effort (can make changes more confidently)
• Prepare the way for Provide data in the application domain rather than the CLI domain #2683 which will be the ultimately solution for this

99% of clap's error reporting is done through debug asserts

• Clap assumes these are development errors, that there isn't a reason to recover from them, and that the majority of testing happens in debug mode (check out App::debug_assert though it doesn't help in this case, and trycmd)
• We limit it to debug for faster smaller binary sizes and faster runtime performance

[epage]

As this is by design and there aren't any more comments, I'm closing this out. If there are any concerns, feel free to raise them