Skip to content

Rename fmt::Writer to fmt::Write #22311

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

Merged
merged 1 commit into from
Feb 17, 2015
Merged

Rename fmt::Writer to fmt::Write #22311

merged 1 commit into from
Feb 17, 2015

Conversation

lambda-fairy
Copy link
Contributor

This brings it in line with its namesake in std::io.

[breaking-change]

r? @aturon

This brings it in line with its namesake in `std::io`.

[breaking-change]
@aturon
Copy link
Member

aturon commented Feb 16, 2015

Sorry for the slow review, was away for the weekend.

This looks great to me! Just want to check in with @alexcrichton before I send it to bors.

@alexcrichton
Copy link
Member

Hm I do agree that Writer seems out of place but to truly mirror io::Write this trait should be called WriteStr as it is the main method of the trait. That seems... odd though.

I would personally be ok just calling this Write despite the primary method being write_str, thoughts @aturon?

@aturon
Copy link
Member

aturon commented Feb 17, 2015

@alexcrichton

I agree, that guideline is intentionally a loose recommendation not a hard rule.

@aturon
Copy link
Member

aturon commented Feb 17, 2015

@alexcrichton

Do you think it's worth going through a deprecation phase for this, or should we just rename as this PR is doing?

@alexcrichton
Copy link
Member

@bors: r+ bc9084b

@alexcrichton
Copy link
Member

I think a rename is fine, I don't think this is a widely used trait at all (unlike fmt::Formatter for example)

@lambda-fairy
Copy link
Contributor Author

@alexcrichton I suspect the reason why WriteStr seems odd is that fmt::Write isn't really a general trait for writing Unicode strings, but a restriction of io::Write that works better with the formatting machinery.

I don't really have an opinion on this though.

@aturon
Copy link
Member

aturon commented Feb 17, 2015

Hm, so we may eventually provide a Unicode-only "textwriter" in IO, and I suppose the name might want to match that -- but I guarantee it won't be called Writer or anything related to it :)

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 17, 2015
 This brings it in line with its namesake in `std::io`.

[breaking-change]

r? @aturon
bors added a commit that referenced this pull request Feb 17, 2015
This brings it in line with its namesake in `std::io`.

[breaking-change]

r? @aturon
@bors
Copy link
Collaborator

bors commented Feb 17, 2015

⌛ Testing commit bc9084b with merge f9aeea7...

@bors bors merged commit bc9084b into rust-lang:master Feb 17, 2015
@lambda-fairy lambda-fairy deleted the consistent-fmt branch June 2, 2016 03:37
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.

4 participants