Fix formatting panic for 0.13.x

[VorpalBlade]

Two commits are needed to fix this on 0.13:

• Do not trace return value (causes panic on Result:Err). This is a cherry pick from main
• Backport changes to Value::string_debug_with. This is manual backport of the way the code is now done in main and fixes the observed crash.

(fixes #747)

[VorpalBlade]

I'm not sure if there are any tests that also need to be backported or not.

[udoprog]

LGTM!

Did you intend to fix the Debug impl for Value as well to not return fmt::Error in case the debug impl errors?

[VorpalBlade]

Did you intend to fix the Debug impl for Value as well to not return fmt::Error in case the debug impl errors?

The fix I did seems to work for my use case (no more panics), but it can still return error indeed. I'm not sure under what conditions that can happen though. But since posting this PR I did manage to hit that path again:

    let cmd = process::Command::new("ls");
    cmd.arg("-l");
    let child = cmd.spawn();
    dbg(cmd);

(This fails because cmd is apparently moved).

So perhaps we should also add something like the <object at address> format for the error case one level up?

[VorpalBlade]

With the latest change we now get debug prints like: <unknown object at 0x7ffd3816a388: Cannot read, value is moved>, which seems maximally informative.

[udoprog]

That's great! I think there's still the is_err thing in the Debug impl (can't check right now) which we should probably get rid off?

Regardless I'll merge this when I'm home. Thanks!

[VorpalBlade]

That's great! I think there's still the is_err thing in the Debug impl (can't check right now) which we should probably get rid off?

I must have missed this, not sure what is left at this point?

[udoprog]

You're right. Thank you!