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

Fixes for compact printing #16842

Merged
merged 2 commits into from
Jun 10, 2016
Merged

Fixes for compact printing #16842

merged 2 commits into from
Jun 10, 2016

Conversation

nalimilan
Copy link
Member

This is a follow-up from #16817. I think the second commit is uncontroversial.

The first one may need more design discussion. Setting :multiline=>false in showcompact sounds logical, but it's slightly annoying that it breaks the direct correspondence with the :compact=>true IOContext property. I don't know whether that's a problem or not. It could be good to rename either the method or the property, but we'd need to find a non-breaking solution.

@@ -1542,9 +1542,18 @@ end
showcompact(x) = showcompact(STDOUT, x)
function showcompact(io::IO, x)
if get(io, :compact, false)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's generally considered a good idea to check whether a flag is set to avoid setting it twice without reason, shouldn't we do this automatically under the hood from IOContext()? That would make this code much shorter.

This provides a simple way of printing the short representation
of a type. Cf. #16817.
Print the wrapped value compactly, as inside other containers,
since the type information is redundant with the type parameter.
Also print the type with :multiline=>false.
@JeffBezanson
Copy link
Member

It's worth noting that this is a bit of a corner case, since it's rare (and actually not a great practice) to define show for specific Types, and even rarer for a type to have a multi-line representation.

@nalimilan nalimilan deleted the nl/showcompact branch June 10, 2016 07:28
@nalimilan
Copy link
Member Author

It's worth noting that this is a bit of a corner case, since it's rare (and actually not a great practice) to define show for specific Types, and even rarer for a type to have a multi-line representation.

Unfortunately, it's rare for types to define this (only Enum does in Base AFAIK), but it's more common to need representing these types, like for Nullable{Enum}. To be fully correct, all places where a type is printed should generally use showcompact, but since it's rare people will likely miss this until somebody tries with an enum. Ah, well...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants