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

pretty print `Debug` of tuples, tuple structs and enum variants in a single line #1198

Closed

Conversation

Projects
None yet
9 participants
@liigo
Copy link
Contributor

liigo commented Jul 10, 2015

Currently, println!("{:#?}", Some(1)); pretty print output as, with newlines and indent:

Some(
    1
)

But I don't think it is pretty print: it's weird and ugly, no one write code like this. It is not a preferred code style also.

Some(1) itself, without adding newlines and indent, is pretty well.

Tuples were already implemented, in 1.0 stable, to pretty print in a single line, without adding newlines and indents. So we should do the same for tuple structs and enum variants, for consistency.

pretty print `Debug` of tuples, tuple structs and enum variants in a …
…single line

Currently, `println!("{:#?}", Some(1));` *pretty* print output as, with newlines and indent:

```
Some(
    1
)
```

I don't think it is **pretty** print: it's weird and ugly, no one write code like this. It is not a preferred code style also.

`Some(1)` itself, without adding newlines and indent, is pretty well.

Tuples were already implemented, in 1.0 stable, to pretty print in a single line, without adding newlines and indents. So we should do the same for tuple structs and enum variants, for consistency.
@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Jul 16, 2015

what if you have a type in the tuple that is printed in multiple lines?

@liigo

This comment has been minimized.

Copy link
Contributor Author

liigo commented Jul 16, 2015

This RFC doesn't change that behavior. Tuples were printed in a single line
in stable Rust already.
On Jul 16, 2015 18:53, "Oliver Schneider" notifications@github.com wrote:

what if you have a type in the tuple that is printed in multiple lines?


Reply to this email directly or view it on GitHub
#1198 (comment).

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jul 18, 2015

Perhaps pretty-printing should keeps more than one thing on the same line up to 80 characters or os. Python’s pprint does something like this.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Jul 18, 2015

As I noted in the related PR rust-lang/rust#26874,

  • The fact that tuples did not pretty print was an oversight that has since been corrected: rust-lang/rust#26913.
  • The pretty printing syntax was designed for readability of large, complex types. The proposed rule impairs readability pretty significantly in that case:
MyTupleStruct(MyStruct {
    a: 1,
    b: TupleStruct(MyOtherStruct {
        foo: bar
    }, 10, "hi", Thing {
        baz: buzz
    })
}, true, false, OtherStruct {
    thing: "hi"
})

An adaptive scheme like @SimonSapin's suggestion would be possible but a bit tricky since this stuff can't allocate. You could do it by pretty-printing fields first into a line and character counting Writer and then printing "for real" based on that.

@arthurprs

This comment has been minimized.

Copy link

arthurprs commented Jul 18, 2015

I'm fine as it is, I'm unsure if we can insert any adaptive behavior without redirecting inner #Debug outputs to a intermediate buffer (which one may argue is a nonstarter).

@jonas-schievink

This comment has been minimized.

Copy link
Member

jonas-schievink commented Jul 18, 2015

I'm in favor of this since I ran into issues with debug printing myself (with code similar to this: http://is.gd/xbWb2l)

The output is either an unreadable oneliner (at least with many opcodes):

[MOV(0, 1), MOV(1, 2), LOADK(2, 1234), SUB(0, 1, 2), ADD(0, 0, 0)]

or this bloated mess:

[
    MOV(
        0,
        1
    ),
    MOV(
        1,
        2
    ),
    LOADK(
        2,
        1234
    ),
    SUB(
        0,
        1,
        2
    ),
    ADD(
        0,
        0,
        0
    )
]

The proposed solution would make this look similar to the vec! in the source code, which would be ideal in this case.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Aug 13, 2015

This RFC is entering its final comment period.

@ProtectedMode

This comment has been minimized.

Copy link

ProtectedMode commented Aug 14, 2015

I agree with @sfackler that this rule would hurt readability in many non-trivial cases. I do not think that making these trivial cases more natural to read justifies making more complex types (in my opinion) much harder to read. Counting line lengths would not even solve this issue, and also add a lot more complexity.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Aug 20, 2015

The @rust-lang/libs team has decided that, while there is something to be said for improving the readability of Debug output, the changes proposed in this PR do not address that issue sufficiently:

  • Readability of tuples and tuple structs containing anything other than primitives, strings, or other tuples and tuple structs is significantly impeded.
  • It's unclear why tuples and tuple structs should be be treated fundamentally different from normal structs - @jonas-schievink's example would be just as relevant if some or all of the variants were struct-like: LOADK { a: 2, b: 1234 }.

An adaptive scheme could be interesting, but will need a lot of work to figure out what we actually mean by "pretty print".

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 27, 2015

The FCP period for this RFC has elapsed, and as detailed in @sfackler's comment the libs team has decided to close. Thanks regardless for the RFC @liigo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.