Skip to content

Conversation

@tbu-
Copy link
Contributor

@tbu- tbu- commented Jan 9, 2015

They just clutter the output, reading

[0i32, 1i32, 2i32, 3i32]

is really hard.

They just clutter the output, reading
```
[0i32, 1i32, 2i32, 3i32]
```
is really hard.
@rust-highfive
Copy link
Contributor

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@huonw
Copy link
Contributor

huonw commented Jan 9, 2015

These suffixes were part of the design fwiw. (I'm not expressing an opinion on whether they should be removed.)

@tbu-
Copy link
Contributor Author

tbu- commented Jan 9, 2015

It's what's written in the design, but it didn't have much discussion. We also don't distinguish between printing Vec, &[], or String, &str, this is just for debugging views of underlying values.

This also negatively affects tests: tbu-/collect-rs@18069a4#diff-8a5ac7b178f2c29d23992135dce5db83R969

@aturon
Copy link
Contributor

aturon commented Jan 9, 2015

FWIW, I'm hoping to post an RFC shortly establishing clearer conventions for fmt::Show and fmt::String. This may be best discussed there.

@tbu-
Copy link
Contributor Author

tbu- commented Jan 9, 2015

It's a pretty clear win in readability, so this could be merged without an RFC (?). Still need to fix stds tests though.

@alexcrichton
Copy link
Member

cc rust-lang/rfcs#565, we may wish to hold off on a decision here until a decision is reached on that RFC

@aturon
Copy link
Contributor

aturon commented Jan 9, 2015

FWIW, the RFC suggests making the change here. I do agree we should wait until that discussion is finished, though, to avoid unnecessary churn.

@alexcrichton
Copy link
Member

Thanks again for the PR @tbu-! The relevant RFC has now been accepted, so this is good to go. I ended up implementing this as well in #21457 (sorry I forgot about this PR!), so I'm going to close this in favor of that to reduce merge conflicts.

bors added a commit that referenced this pull request Jan 24, 2015
lnicola pushed a commit to lnicola/rust that referenced this pull request Oct 14, 2025
…ct-ty-alias

minor: Fix using `make::ty` for extract_type_alias
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants