Skip to content

remove io :multiline attribute#17113

Merged
JeffBezanson merged 1 commit intomasterfrom
jn/show-multiline
Jun 29, 2016
Merged

remove io :multiline attribute#17113
JeffBezanson merged 1 commit intomasterfrom
jn/show-multiline

Conversation

@vtjnash
Copy link
Copy Markdown
Member

@vtjnash vtjnash commented Jun 25, 2016

this attribute doesn't nest properly,
and functions just end up branching almost the entire code based
on this parameter, so it's cleaner as a separate method

fix #17087
fix #16910
fix #17090

@vtjnash vtjnash added the io Involving the I/O subsystem: libuv, read, write, etc. label Jun 25, 2016
eval(Multimedia, quote
export @MIME
macro MIME(s)
Base.warn_once("@MIME(\"\") is deprecated, use MIME\"\" instead.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

apparently this has been deprecated for almost 3 years, just was never in this file - I think it would be safe to just delete it? af8c18c

@vtjnash vtjnash force-pushed the jn/show-multiline branch from 262230d to 32003dd Compare June 25, 2016 19:43
end
end
end
show(io::IO, iter::Union{KeyIterator,ValueIterator}) = show(io, collect(iter))
Copy link
Copy Markdown
Contributor

@tkelman tkelman Jun 25, 2016

Choose a reason for hiding this comment

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

this will be showing them as if they're a different type than they actually are? should at least have a header?

edit: or what determines when this gets called vs the mime version in replutil?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it'll show them in compact form without a header (ie. when you call show directly, instead of displaying a mimetype)

@JeffBezanson
Copy link
Copy Markdown
Member

This seems to make it impossible to implement single and multiline output for different mime types.

@vtjnash
Copy link
Copy Markdown
Member Author

vtjnash commented Jun 25, 2016

Can you give an example? This doesn't prevent you from making writers for other mimetypes.

@JeffBezanson
Copy link
Copy Markdown
Member

IIUC, this puts multiline output in methods with mimetypes, and single line output in methods with 2 arguments. So how do you define both single and multiline output for html?

@vtjnash
Copy link
Copy Markdown
Member Author

vtjnash commented Jun 26, 2016

IIUC, this puts multiline output in methods with mimetypes, and single line output in methods with 2 arguments. So how do you define both single and multiline output for html?

By using the (not yet provided by a package) Builder-type objects. This change defines the mime-type methods as outputting documents, and the show methods as outputting strings. The overlap between the two would be handled by the IO object, when outputting a mime-aware value into a document-aware context.

@JeffBezanson
Copy link
Copy Markdown
Member

OK, I think where we ended up on this is to do this change for now, and if we need to implement single-line outputs for, say, HTML, that can be expressed by passing a display height of 1 in the IOContext.

@kshyatt
Copy link
Copy Markdown
Member

kshyatt commented Jun 28, 2016

I've restarted the AV build. @vtjnash if it passes, do you want to rebase and see if that passes CI, and then merge?

@JeffBezanson
Copy link
Copy Markdown
Member

Rebased.

this attribute doesn't nest properly,
and functions just end up branching almost the entire code based
on this parameter, so it's cleaner as a separate method

fix #17087
fix #16910
fix #17090
@JeffBezanson JeffBezanson merged commit 98d49ab into master Jun 29, 2016
@tkelman tkelman deleted the jn/show-multiline branch June 29, 2016 15:16
omus added a commit to omus/julia that referenced this pull request Jun 29, 2016
omus added a commit to omus/julia that referenced this pull request Jul 7, 2016
omus added a commit to omus/julia that referenced this pull request Jul 8, 2016
omus added a commit to omus/julia that referenced this pull request Jul 11, 2016
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
end
end

show(io::IO, ::MIME"text/plain", X::AbstractArray) = showarray(io, X, false)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vtjnash Following up on my other comment. Why did you define these methods for MIME"text/plain" instead of two-argument show as #16563 prescribes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

io Involving the I/O subsystem: libuv, read, write, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macroexpand display regression Display regression for tuples of arrays Messy @test output in case of success since IOContext changes

5 participants