fix(complete): Use "expect" instead of "unwrap"

[hasezoey]

Use expect instead of unwrap to provide context in case it does fail

fixes #3319

---

i know that @epage said at #3319 (comment)

The reason the error message isn't better is that it isn't intended for this case. Same for why the bin_name is required

but i still think it is better to use expect instead of unwrap to get a better debugging experience in case it does fail (like accidental misuse in my case)

[hasezoey]

updated to fix rustfmt

[epage]

This reads like its describing app.get_bin_name().unwrap() and isn't adding any value.

A tip for writing expects is to use them to document what the expected pre-condition is that failed. In this case, crate::generate set the bin_name

[hasezoey]

and isn't adding any value.

i mean its still better than unwrap failed at, this way you currently have at least the clue what property is missing before having to investigate the code that threw the message

but yes, the message is not really great

how about changing the message to:
Getting the App's \"bin_name\" failed: did crate::generate set the \"bin_name\" correctly?

[epage]

The distinction is expects are not error reporting but a form of assertion. Developers get a traceback telling them where it failed and can easily see what the line is that failed. The emphasis should be on the documentating the invariant (by emphasis, I mean linguistically)

This goes back to the tip I mentioned. A more concrete way of putting it is to turn "expect("...")into "expect ..." and fill in the sentence fragment. In this case "expect crate::generate set the bin_name" which becomesexpect("crate::generate set the bin_name")`. Yes, the panic message isn't optimized for this (its a common complaint of expect's panic message).

[hasezoey]

expect crate::generate set the bin_name
crate::generate set the bin_name

they dont sound descriptive to me, how about Expected crate::generate to have set "bin_name" and maybe with the addition of (Did this function get run directly?) as a hint to not run this function directly (but through crate::generate)

[epage]

Expected crate::generate to have set "bin_name"

As I said, this is redundant with the line that is failing the assert

(Did this function get run directly?)

This is more of error reporting verbage than assertion violation verbage.

[epage]

To move this forward, I've gone ahead and created #3328

[epage]

btw some of these problems will hopefully go away when we resolve #2911.