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

format_into() requires io::Write; what about fmt::Write in Debug/Display impls? #375

Open
aldanor opened this issue Oct 26, 2021 · 11 comments
Labels
A-formatting Area: formatting C-blocked Category: blocked by another (possibly upstream) issue C-feature-request Category: a new feature (not already implemented)

Comments

@aldanor
Copy link

aldanor commented Oct 26, 2021

Simple (and the most common, I believe) use case - you're implementing Display or Debug for your type containing some time objects, and you are provided with std::fmt::Formatter which implements std::fmt::Write trait. (maybe I'm missing something trivial, so apologies in advance if that's the case)

Current format_into implementations though require &mut impl std::io::Write which is a different beast, more complex and aimed at different stuff, with vectored writes, flushing and all the rest. If one were to choose between the two, wouldn't it make sense to support fmt::Write and not io::Write? Is there a clean alternative way?

(There's crates like fmt2io that provide hacky bridges but that's just ugly and wrong)

@jhpratt
Copy link
Member

jhpratt commented Oct 26, 2021

@CryZe messaged me yesterday about this, actually. He's working on a PR that would do this in a backwards-compatible way.

@jhpratt jhpratt added A-formatting Area: formatting C-feature-request Category: a new feature (not already implemented) labels Oct 26, 2021
@mzabaluev
Copy link
Contributor

mzabaluev commented Nov 25, 2021

It would be nice to make format (which only requires alloc) and any future formatting methods using core::fmt::Write available in no-std. However, the implementation and feature gate for format is coupled with format_into which depends on std::io::Write.

To summarize, the three use cases can each be made available behind different feature gates:

Method Features
format alloc
format_into std
format_into_fmt_writer

@jhpratt
Copy link
Member

jhpratt commented Nov 25, 2021

The primary problem that formatting into fmt::Write is that arbitrary bytes can be present in the format description. It's not immediately clear how this interacts with an output that requires UTF-8.

That said, the existing format method can not change as it would be breaking to do so.

@mzabaluev
Copy link
Contributor

That said, the existing format method can not change as it would be breaking to do so.

No, but it could be made available under a formatting-alloc feature gate that would only imply alloc, and would be auto-enabled by full formatting.

@jhpratt
Copy link
Member

jhpratt commented Nov 25, 2021

It has to be under a different method name. Feature gates can't change a method signature.

@mzabaluev
Copy link
Contributor

Feature gates can't change a method signature.

Sorry about the possible confusion, but I'm not asking to modify the signature of format. Just enable it under a feature gate that does not pull in std.

I think this proposal needs to be discussed as its own issue; formatting into fmt::Write is somewhat relevant because it's another case where std is not required, but also alloc isn't.

@mzabaluev
Copy link
Contributor

The primary problem that formatting into fmt::Write is that arbitrary bytes can be present in the format description. It's not immediately clear how this interacts with an output that requires UTF-8.

That's got to be already solved for format which must end up with a String, hasn't it?

@jhpratt
Copy link
Member

jhpratt commented Nov 25, 2021

I'm not asking to modify the signature of format. Just enable it under a feature gate that does not pull in std.

This inherently requires a new method, as the existing signature requires std.

That's got to be already solved for format which must end up with a String, hasn't it?

Right now String::from_utf8_lossy is used — which requires std.

@mzabaluev
Copy link
Contributor

I'm not asking to modify the signature of format. Just enable it under a feature gate that does not pull in std.

This inherently requires a new method, as the existing signature requires std.

Ah, but changing the return type to alloc::string::String would not break the signature, as it's the same type.

Right now String::from_utf8_lossy is used — which requires std.

Understood, it will be more difficult with incremental output and no allocations.

@jhpratt
Copy link
Member

jhpratt commented Nov 25, 2021

Hmm...you may be right after ~30 seconds of looking at code. It may be possible to have format only require alloc. However, this would require behavior identical to current, which will not be easy to accomplish. Some things could be gated internally that aren't right now.

@jhpratt
Copy link
Member

jhpratt commented Mar 10, 2022

As I just mentioned in #407, my current intent is to wait for min_specialization. This will permit either io::Write or fmt::Write to be passed as a parameter, preferring io::Write if the value implements both. Marking this issue as blocked for on min_specialization.

@jhpratt jhpratt added the C-blocked Category: blocked by another (possibly upstream) issue label Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-formatting Area: formatting C-blocked Category: blocked by another (possibly upstream) issue C-feature-request Category: a new feature (not already implemented)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants