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

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

Closed
EliasHolzmann opened this issue Oct 15, 2023 · 4 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@EliasHolzmann
Copy link
Contributor

EliasHolzmann commented Oct 15, 2023

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. Allow modifying Formatter members rust#19207
  2. Feature request: Add a way to capture a formatter that can be passed to format helper functions rust#74870
  3. Unable to create Formatter 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: rust-lang/rfcs#3394 (comment)

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):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@EliasHolzmann EliasHolzmann added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Oct 15, 2023
@dtolnay
Copy link
Member

dtolnay commented Oct 16, 2023

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

@EliasHolzmann
Copy link
Contributor Author

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
Copy link
Member

m-ou-se commented Oct 19, 2023

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
Copy link
Contributor Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants