rust-lang / libs-team

The home of the library team
Apache License 2.0
115 stars 18 forks source link

[ACP] Provide an interface for creating and modifying instances of fmt::Formatter #280

Closed EliasHolzmann closed 10 months ago

EliasHolzmann commented 10 months ago

Proposal

Problem statement

Creating a custom Formatter at runtime as well as modifying an existing Formatter is currently not supported, one can only use the Formatters passed in by the formatting macros. Being able to create and modify them could be useful for multiple use cases:

  1. Calling the formatting code of contained fileds with a modified formatting parameter
  2. Exposing different formatting methods for different representations/data points on one struct
  3. Exposing formatting methods that require additional data
  4. Writing libraries that provide enhanced formatting capabilities

Motivating examples or use cases

  1. rust-lang/rust#19207
  2. rust-lang/rust#74870
  3. rust-lang/rust#46591
  4. Multiple instances fit this use case:
    • runtime-fmt (a formatting crate that allows the user to supply the format string at runtime) seems to use the unstable fmt_internals feature to build a custom std::fmt::Arguments which has the Formatter baked in (or rather the mostly equivalent fmt::rt::Placeholder struct), see here. In consequence, this crate requires nightly Rust. (Note that the interface isn't the current one as runtime-fmt hasn't been updated since 2019)
    • rt_format (another runtime formatting crate) handles the problem in another way: Using a macro named generate_code!, it generates a function containing a format_args! call for every combination of alignment, signedness, default/alternative representation, zero/space padding, width (via variable), precision (via variable) and formatting trait for a total of 1024 format_args! invocations at the time of writing. Fill characters are not supported, as those cannot be passed via a variable to the format_args! call but must be part of the format specifier. (If you are interested to see the result of this approach, run cargo expand in the crate root and search for a huge function named format_value)
    • I'd like to experiment with a crate that reimplements the formatting macros but adds some additional features (mostly interpolation). However, it is currently impossible to do this in a manner that is compatible with the std implementation outside of nightly Rust (rt_format is is almost there, but they cannot support fill characters, apart from their solution being quite hacky and probably inefficient).

Solution sketch

  1. Add a new function fn new(write: &'a mut (dyn Write + 'a)) -> Formatter<'a> to Formatter that constructs a new instance of Formatter that is equivalent to the {} formatting specifier (no flags, filled with spaces, no alignment, no width, no precision).
  2. Add new methods set_sign_plus/set_sign_minus/set_alternate/set_sign_aware_zero_pad/set_fill/set_align/set_width/set_precision to Formatter that set the field to the indicated value.
  3. Add a new method fn clone(&'a mut self) -> Self to Formatter that clones the instance. Note that this is not a impl of the Clone trait as this method takes a mutable borrow of self (instead of an immutable one). The mutable borrow is held for the lifetime of the clone. This is necessary because the cloned instance holds the same mutable reference to the output Write instance as the instance it was cloned from.

Alternatives

Another idea I had was to use the builder pattern to build new Formatters. The drawback I see with the builder pattern is that changing a single parameter, as needed in use case 1 above, wouldn't be possible. The user would instead need to create a new builder from the existing Formatter:

let fmt = FormatterBuilder::from(fmt).set_width(Some(42)).build();

This is quite verbose compared to the proposed solution:

fmt.set_width(Some(42));

On the other hand, the prposed solution adds 10 functions to Formatter that most users do not care about and that make the documentation harder to navigate. I am still a bit on the fence about which way to go here – feedback is appeciated.

Also, this proposal passes the output stream as a dyn Trait to the new function. If in the future, Formatter gets refactored to be generic struct with the output struct used as a generic type parameter, fn new will not match this new interface. However,

  1. I don't see how the proposal could be implemented instead, and
  2. I don't believe refactoring Formatter in this fashion is possible anyway – this would change the interface and therefore would be a breaking change.

Links and related work

Relevant previous libs team discussion: https://github.com/rust-lang/rfcs/pull/3394#issuecomment-1563263573

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution:

dtolnay commented 10 months ago

Relevant previous libs team discussion: https://github.com/rust-lang/rfcs/pull/3394#issuecomment-1563263573

EliasHolzmann commented 10 months ago

Relevant previous libs team discussion: rust-lang/rfcs#3394 (comment)

Thanks, I had a look through all open and accepted RFCs but didn't look through closed ones. Added the link to the ACP.

m-ou-se commented 10 months ago

The main takeaway from that previous discussion is that we should have an interface for creating a fmt::Formatter with custom options, not for modifying a fmt::Formatter to change formatting options.

Allowing a change for formatting options through &mut fmt::Formatter would allow implementations of e.g. Display and Debug to change the options to its argument, which is not something we can properly support or define. For example, std::fmt::write currently assumes that that the Display/Debug/...'s fmt functions do not change the options, such that the Formatter can be re-used if no options were specified in the format string.

So, a possible solution could be having a new function that takes options, or having a builder type, or something else in that direction.

EliasHolzmann commented 10 months ago

Thanks! After thinking a bit about this, I agree that allowing to modify Formatters is a bad idea that would break existing expectations – apart from the problems with std::fmt::write and possibly other parts of the std lib, there would also be the possibility of external crates that may rightfully expect that having a Display impl call into the the Display impl of some struct member cannot change the Formatter.

I'm gonna close this ACP now and open another one with with a new proposal in a few days (probably at the weekend). Thanks again!