Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DisplayAsDebug - A struct to help with Debug impls #49067

Closed
wants to merge 11 commits into from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Mar 15, 2018

This adds a simple wrapper:

pub struct DisplayAsDebug<T>(pub T);

impl<T: fmt::Display> fmt::Debug for DisplayAsDebug<T> {
    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
        <T as fmt::Display>::fmt(&self.0, fmt)
    }
}

r? @alexcrichton

@Centril Centril added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 15, 2018
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2018
@sfackler
Copy link
Member

We can be a bit more flexible here and be generic over T: Display.

@Centril
Copy link
Contributor Author

Centril commented Mar 15, 2018

@sfackler Interesting...! so rename to DebugDisplay?

@Centril
Copy link
Contributor Author

Centril commented Mar 15, 2018

And now we are more flexible with DebugDisplay ;)

@Centril Centril changed the title Add DebugStr - A struct to help with Debug impls Add DebugDisplay - A struct to help with Debug impls Mar 15, 2018
@sfackler
Copy link
Member

I've written this a couple of places when I've needed to do something like this. It's a tiny little utility, but I think nice to have.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Mar 17, 2018

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 17, 2018
@SimonSapin
Copy link
Contributor

Bikeshed: DebugAsDisplay?

@sfackler
Copy link
Member

Yeah, I was kind of thinking that an As in the middle would be good as well.

@Centril
Copy link
Contributor Author

Centril commented Mar 18, 2018

Totally agree with the As bit;

Per discussion on #rust-libs I went with DisplayAsDebug since you are "converting" a Display impl in effect to a Debug impl (at least I think that reads somewhat more naturally than the reverse).

I took the liberty of making a tracking issue now so I don't have to make a commit again ^,-

@Centril Centril changed the title Add DebugDisplay - A struct to help with Debug impls Add DisplayAsDebug - A struct to help with Debug impls Mar 18, 2018
@kennytm
Copy link
Member

kennytm commented Mar 18, 2018

Why can't you use format_args! for this

use std::fmt::{Debug, Formatter, Result};

struct Arm<'a, L: 'a, R: 'a>(&'a (L, R));
struct Table<'a, K: 'a, V: 'a>(&'a [(K, V)], V);

impl<'a, L: 'a + Debug, R: 'a + Debug> Debug for Arm<'a, L, R> {
    fn fmt(&self, fmt: &mut Formatter) -> Result {
        L::fmt(&(self.0).0, fmt)?;
        fmt.write_str(" => ")?;
        R::fmt(&(self.0).1, fmt)
    }
}

impl<'a, K: 'a + Debug, V: 'a + Debug> Debug for Table<'a, K, V> {
    fn fmt(&self, fmt: &mut Formatter) -> Result {
        fmt.debug_set()
           .entries(self.0.iter().map(Arm))
           .entry(&Arm(&(format_args!("{}", "_"), &self.1)))
           .finish()
    }
}

fn main() {
    let table = (1..3).enumerate().collect::<Vec<_>>();
    assert_eq!(format!("{:?}", Table(&*table, 0)),
               "{0 => 1, 1 => 2, _ => 0}");
}

@alexcrichton
Copy link
Member

@rfcbot concern exists

Thanks @kennytm! Looks like this may already exist in libstd?

@Centril
Copy link
Contributor Author

Centril commented Mar 18, 2018

Some thoughts:

  1. I had no idea that Arguments had this behavior -- so this was entirely unexpected. Is this a guarantee of format_args! / Arguments to behave in this way tho (or just a happy accident)? If so - it should be documented somewhere.

  2. The specific case above can be shortened to: format_args!("_").

  3. There's a minor overhead: 16 bytes vs. 48 -- it's not much (so it probably does not warrant an extra type), but I thought I'd mention it.

@SimonSapin
Copy link
Contributor

Interesting! I think I’m now in favor of closing this RFC and either adding a new example or modifying existing examples to show this use of format_args! in the docs of fmt::Formatter::debug_{struct,tuple,list,set,map} and fmt::Debug{Struct,Tuple,List,Set,Map}.

@dtolnay
Copy link
Member

dtolnay commented Mar 18, 2018

👍 for directing people to format_args! for this use case. @rfcbot fcp close

@rfcbot
Copy link

rfcbot commented Mar 18, 2018

@dtolnay proposal cancelled.

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 18, 2018
@rfcbot
Copy link

rfcbot commented Mar 18, 2018

Team member @dtolnay has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 18, 2018
@Centril
Copy link
Contributor Author

Centril commented Mar 18, 2018

I'll expedite the process and close the PR myself ;)

@Centril Centril closed this Mar 18, 2018
@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 18, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
…ug, r=steveklabnik

Document format_args! / Arguments<'a> behavior wrt. Display and Debug

This is a follow up PR to rust-lang#49067 , this documents the behavior of `format_args!` (i.e: `Argument<'a>`) wrt. `Display` and `Debug`.
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
…ug, r=steveklabnik

Document format_args! / Arguments<'a> behavior wrt. Display and Debug

This is a follow up PR to rust-lang#49067 , this documents the behavior of `format_args!` (i.e: `Argument<'a>`) wrt. `Display` and `Debug`.

r? @steveklabnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants