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

#[deriving(Show)] handles its interior fundamentally incorrectly #12128

Closed
huonw opened this Issue Feb 9, 2014 · 10 comments

Comments

Projects
None yet
5 participants
@huonw
Copy link
Member

huonw commented Feb 9, 2014

e.g.

#[deriving(Show)]
struct Foo {
    x: ~str
}

fn main() {
    println!("{}", Foo { x: ~"bar }" });
}

prints Foo { x: bar } }, but Foo { x: ~"bar }" } or Foo { x: "bar }" } would be far more reasonable.

Our Show differs from Haskell's (which also has deriving (Show)) in that show string puts quotes around it, while our {} is puts it straight inline (i.e. they're actually different operations).

Proposal: create (yet another) formatting trait Repr and replace #[deriving(Show)] with Repr. This would mean that println!("{:r}", "foo") would be printed as "foo" rather than just foo like {} does. I feel like this is a more accurate description of what it's doing: it's a raw representation of a type, while {} is meant to be more polished.

@huonw

This comment has been minimized.

Copy link
Member Author

huonw commented Feb 9, 2014

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 9, 2014

Another option we have is the # modifier. This essentially means print the "alternate output" which is defined per-type being formatted. For example {:#x} will print a hexadecimal number with a leading 0x. Perhaps the formatter for strings could recognize the # modifier and print quotes around it?

Additionally formatting for ~T with # could put a ~ in front (maybe)

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 12, 2014

cc me

@huonw

This comment has been minimized.

Copy link
Member Author

huonw commented Mar 12, 2014

@alexcrichton that sounds reasonable; would you think that a deriving(Show)'d type would use the # format internally when formatted with both {} and {:#}?

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 12, 2014

one potential advantage of a separate trait rather than using the # modifier is that other libraries implementing Show may forget to thread the # through when rendering its substructures.

E.g. the sample code illustrating fmt::Show has this:

impl fmt::Show for Vector2D {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f.buf, "({}, {})", self.x, self.y)
    }
}

this would have to be changed to something like:

    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        if self.flags & (1 << FlagAlternate) {
            write!(f.buf, "({:#}, {:#})", self.x, self.y)
        } else { 
            write!(f.buf, "({}, {})", self.x, self.y)
        }
    }

which may not be the most obvious thing to write.

But then again, if deriving(Show) does the right thing (in terms of passing the flag down by some means or another) then maybe this worry is unwarranted.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 12, 2014

A more principled way of stating my previous comment is: How much code does one expect to be shared between the two control paths?

If sharing is unlikely, then two distinct traits seems sensible.

But I actually can believe that there is potential for much sharing, depending on how one architects one's implementation of Show. So I would not object to using the FlagAlternate for this purpose.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 12, 2014

I don't expect many implementations to custom handle the FlagAlternate flag. I would envision it being checked in the primitive implementations, but not much else.

Most formatting flags are completely ignored for all but the primitive implementations anyway, so I wouldn't imagine that starting to recognize the # modifier would change the state too much.

@lilyball

This comment has been minimized.

Copy link
Contributor

lilyball commented Oct 6, 2014

I've long thought that it makes sense to support {:#} to get a string repr output (and, presumably, repr outputs of other basic types, e.g. 10u for a uint).

I think it's fine to put this into Show. Most types won't need to care about the distinction, as most types tend to print repr output anyway. Derived Show impls would probably want to use {:#} as the format token for the fields.

I am mildly concerned about the inability to easily propagate this flag in manually-written Show impls. However, given that very few Show impls would bother even checking the flag, it's probably not a big deal.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Oct 7, 2014

If we had the scoped task locals (aka thread-local fluids, aka dynamic parameters) that we have occasionally discussed (and that the old condition system embedded), then we could use that for the {:#} bit and then the threading would be implicit.

I wonder if there is an issue somewhere for this (library) feature

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Feb 15, 2015

Today, the equivalent code with Debug prints

Foo { x: "bar }" }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.