Show impl for RefCell

[forticulous]

Safe Show impl for RefCell.

I'm open to debate about how to handle the case where try_borrow() fails. I went with displaying that the refcell is borrowed to minimize surprises

[lilyball]

I don't think it's appropriate to have Show on RefCell. We have no precedent for a Show impl that can fail, and I think it's a really bad idea to introduce one. The user can call .borrow() or .try_borrow() themselves and rely on the Show impls you already wrote for Ref/RefMut.

[forticulous]

I think Show impl is useful for RefCell because more often than not, RefCell is going to be used in complex data structures rather than alone. And in order to show everything it's going to need a Show impl.

Also this impl doesn't fail, the test makes sure it doesn't. If you mean it might print out something different depending on the state of RefCell then yes it will do that, but it makes sense given what RefCell is all about.

What it displays when the refcell is already borrowed can easily be changed. I was thinking either show nothing or some kind of message saying it's already borrowed, and the latter is less confusing so I went with that

[lilyball]

Ok yes, this isn't failure in the sense of task failure. but it is failure in the sense that it's unexpectedly showing useless information.

[forticulous]

The other option would be something like this:

match self.try_borrow() {
    Some(_) => (*self.borrow()).fmt(f),
    None => Err(WriteError)
}

[lilyball]

Well, this is why I think it's better to let the client figure out how to handle dynamic borrow failures. Which means let them write Show themselves.

[forticulous]

Incidentally it would be nice if there were another implementation of FormatError like WriteError that would have a message about why the format failed

[forticulous]

@alexcrichton What do you think?

[alexcrichton]

I agree, RefCell is fallible, it should not implement Show. Whatever decision is made regarding how to format it will inevitably be the wrong decision at some point.

[forticulous]

The Show trait does return a Result type which allows for the possibility of failure though right? Although there only way to fail is by using WriteError that doesn't convey any information about the nature of the failure at the moment.

[forticulous]

Also, I agree that users may want to override it to suit their needs, but there should be some default implementation. Otherwise the only way to print out a complex datastructure with RefCells in it is to make your own wrapper type as far as I know

[lilyball]

@forticulous The error result from Show is intended to represent an error in actually writing to the destination, not an error in constructing the textual representation of the value. Before the libstd split, it used to return an IoError instead of this custom fmt::FormatError, which made it more obvious that it was only intended for I/O issues.

Which is to say, using Show to write to an in-memory buffer should never fail.

[alexcrichton]

If the Show implementation were to exist, I would implement it basically as-is.