Add some missing Show implementations in libstd#12218
Add some missing Show implementations in libstd#12218brendanzab wants to merge 1 commit intorust-lang:masterfrom
Conversation
There was a problem hiding this comment.
Could this delegate to the above one?
|
Why are all of these marked |
src/libstd/ascii.rs
Outdated
There was a problem hiding this comment.
Perhaps (self.chr as char).fmt(f) ?
There was a problem hiding this comment.
self.chr is a u8
There was a problem hiding this comment.
With small primitives like this, though, it's nice to take the formatting flags into account (wherever possible). This is losing the formatting flags by writing directly, but if you call fmt(f) it'll take all the formatting flags into account properly (delegating elsewhere)
There was a problem hiding this comment.
Ohhh! Woops, for some reason I forgot that u8 as char was allowed.
|
Updatated! |
There was a problem hiding this comment.
Did you actually run this test? I don't see how it can possibly pass. You're asserting that your ShowableStructs will print their values as s2, s4, but AFAIK they'll print as ShowableStruct { value: 2 }, etc.
There was a problem hiding this comment.
Ah woops. Fixing.
There was a problem hiding this comment.
My recommendation is to format your two structs directly into variables, and then construct the expected values using that, so you're not dependent upon the implementation of #[deriving(Show)].
|
Oh deary. |
src/libstd/vec.rs
Outdated
There was a problem hiding this comment.
This should just be
self.as_slice().fmt(f)|
Please don't r+ just yet, gotta fix some things and re-run the tests. |
|
Nice work! r=me with the tests fixed |
src/librustdoc/html/format.rs
Outdated
There was a problem hiding this comment.
This might be simpler if you defined
struct Arguments(~[clean::Argument]);and implemented fmt::Show on that instead. Besides being perhaps cleaner, it also matches the previous behavior, whereas this function here will actually apply any padding/flags from the formatter to the string as a whole.
There was a problem hiding this comment.
Yeah, that's probably a better way. Thanks!
|
r=me if you change the |
Document manifest options
All built-in cargo commands have this output included when run with `cargo foo --help`. Clippy is the only exception, so I have copied the text here. As far as I can tell, the flags come from Cargo itself and not clippy, but the user almost certainly does not care about that.
```text
Manifest Options:
--manifest-path <PATH> Path to Cargo.toml
--frozen Require Cargo.lock and cache are up to date
--locked Require Cargo.lock is up to date
--offline Run without accessing the network
```
changelog: Add the standard Manifest Options section to the `--help` output
No description provided.