rust-lang / libs-team

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

ACP: Add `finish_non_exhaustive` to `DebugList`, `DebugMap`, and`DebugTuple` #248

Closed tgross35 closed 2 months ago

tgross35 commented 1 year ago

Proposal

Problem statement

Many formatting implementations may want to print only a subset of items from a list, set, or map. Subsets can be printed directly (e.g., .item(my_slice[..10])) but there is no clear way to indicate that there are more elements not dislayed.

Motivating examples or use cases

Currently, one of the best solutions to showing non exhaustiveness looks as follows, in this simple wrapper type that prints up to 6 elements in a buffer:

struct TruncatedPrinter<'a, T>(&'a [T]);

impl<'a, T: fmt::Debug> fmt::Display for TruncatedPrinter<'a, T> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        // print at most 6 elements (don't ever omit the `..`, for simplicity)
        let to_print = min(self.0.len(), 6);
        f.debug_list().entries(self.0[..to_print]).entry(&{..}).finish()
    }
}

The output looks like the following:

[1, 2, 3, 4, 5, 6, ..]

This is correct, but (1) is less clear and less discoverable than finish_non_exhaustive, and (2) relies on the debug implementation of RangeFull (though this is unlikely to change).

Solution sketch

This proposes adding a finish_non_exhaustive method to other DebugX types, to mirror what is already possible with DebugStruct. New API would be:

impl<'a, 'b: 'a> DebugList<'a, 'b> {
    pub fn finish_non_exhaustive(&mut self) -> Result<(), Error>;
}

impl<'a, 'b: 'a> DebugMap<'a, 'b> {
    pub fn finish_non_exhaustive(&mut self) -> Result<(), Error>;
}

impl<'a, 'b: 'a> DebugTuple<'a, 'b> {
    pub fn finish_non_exhaustive(&mut self) -> Result<(), Error>;
}

Each of these results should add an element .. to the output list, similar to DebugStruct's behavior.

Alternatives

An alternative could be to provide a NonExhaustive element that implements Debug and prints .., which could be passed to entry. This would have the advantage of giving an easy way to omit items in the middle of a list, e.g. [1, 2, ..., 6], and would be more clear than the status quo &{..}.

This option was not included in order to match DebugStruct's precedent.

Links and related work

Existing API: DebugStruct::finish_non_exhaustive

debug_non_exhaustive discussion and implmentation: https://github.com/rust-lang/rust/issues/67364

That issue does discuss adding the implementation for other DebugX types, but I do not see any specific reason that they were excluded outside of keeping the MVP minimal. There are a few comments expressing desire for DebugTuple to provide this method.

What happens now?

This issue is part of the libs-api team API change proposal process. 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:

tgross35 commented 1 year ago

I wasn't sure whether this would need an ACP since https://github.com/rust-lang/rust/issues/67364 didn't have any specific proposal, but figured it doesn't hurt before doing the work.

pitaj commented 1 year ago

I think we should heavily discourage abbreviating the debug output of map or list. I'm not sure if making that easier is the right course of action.

tgross35 commented 1 year ago

Why should it be discouraged? When I have types that contain large slices, I much prefer their standard debug output only print 20 relevant bytes rather than a whole 100kB+ buffer. It makes it easier to see the other fields while still giving a hint about the content.

Same thing for messages related to maps or sets, where you want to indicate a few special or problamatic items but everything else is OK.

pitaj commented 1 year ago

Personally, I want my debug output to include all the information available. Hiding information just because there's a lot of it seems antithetical to the whole idea of debug.

Like if I'm trying to see what's in some buffer, I don't want it to just show me the first few bytes.

tgross35 commented 1 year ago

I would counter that if I'm trying to see what's in some buffer, I would print my_struct.buffer specifically. But if I'm trying to see any of the other fields in my_struct, I would rather see a truncated buffer than either (1) no field at all, or (2) fill up my terminal scrollback printing one field (especially in pretty printing mode, one line per byte is rough). I'm not suggesting truncated printing become the default, just that it would be nice if there's an easy way to indicate that there's more data not shown.

But I've been working with large blobs recently, so I admit the need isn't in the most common workflow.

CAD97 commented 1 year ago

The existence of this API doesn't particularly encourage eliding details from the Debug representation. Custom implementations of Debug can and already do omit some information for clarity. Debug generally isn't intended to display/debug the internal structure, but the outer data "shape," and in that context, too much information can easily obscure the more useful higher level structure. What's really wanted is a foldable inspector like typically provided by debugger views, including toggle between high-level and low-level details of the inspected type.

For clarity, there's no suggestion to use this for any std types at the moment, and these are just provided for downstream impl Debug. #[non_exhaustive] is permitted on tuples, so at a minimum finish_non_exhaustive should be available on DebugTuple.

If it's considered important, the method documentation can carry an admonition not to use it to arbitrarily hide data that would likely be useful to the containing context, and to consider checking alt mode ({:#?}) and not eliding data in that case.


For current stable, instead of &{..} (ab)using RangeFull, using &format_args!("..") is another option, and a useful trick to get an adhoc Display + Debug object.

Future work could be allowing precision specifiers et al to control how the Debug* format helpers present data.

Amanieu commented 2 months ago

We discussed this in the libs-api meeting and we are happy to add these. A similar method should also be added to DebugSet.