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

Add custom MIME types to display_dict. #755

Merged
merged 7 commits into from
Oct 9, 2018

Conversation

twavv
Copy link
Contributor

@twavv twavv commented Oct 7, 2018

Fixes #749.

I think this is especially prudent given JupyterLab's focus on implementing custom MIME types rather than just rendering everything as HTML with JavaScript.

Very much open to comments (especially with regards to style and best practices!) and critiques.

@twavv twavv changed the title [WIP] Add custom MIME types to display_dict. Add custom MIME types to display_dict. Oct 7, 2018
@twavv
Copy link
Contributor Author

twavv commented Oct 7, 2018

Ping @stevengj

Could you please take a look at this?

MIME("text/plain"),
MIME("image/svg+xml"),
MIME("image/png"),
MIME("image/jpeg"),
Copy link
Member

Choose a reason for hiding this comment

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

png and jpeg should be in a vector to have the same behavior as before

[MIME("application/vnd.vegalite.v2+json"), MIME("application/vnd.vega.v3+json")],
])

register_ijulia_mime(x::Union{MIME, Vector{MIME}}) = push!(ijulia_mime_types, x)
Copy link
Member

Choose a reason for hiding this comment

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

No ijulia here since that is already in the module name. i.e. IJulia.register_mime

@twavv
Copy link
Contributor Author

twavv commented Oct 8, 2018

@stevengj I believe I've resolved all of those

src/execute_request.jl Outdated Show resolved Hide resolved
@twavv
Copy link
Contributor Author

twavv commented Oct 9, 2018

Done! :^)

src/execute_request.jl Outdated Show resolved Hide resolved
src/execute_request.jl Outdated Show resolved Hide resolved
@twavv
Copy link
Contributor Author

twavv commented Oct 9, 2018

😄 Thanks for the guidance!

@stevengj stevengj merged commit 5488fc6 into JuliaLang:master Oct 9, 2018
@stevengj
Copy link
Member

stevengj commented Oct 9, 2018

Thanks for persisting on this!

@twavv
Copy link
Contributor Author

twavv commented Oct 9, 2018

Could you do a minor release so that I can depend on this in WebIO?

@stevengj
Copy link
Member

stevengj commented Oct 9, 2018

I was hoping to merge #756 first once it is ready, but I will check back later today.

@stevengj
Copy link
Member

tagged 1.13

cnliao pushed a commit to cnliao/IJulia.jl that referenced this pull request Oct 10, 2018
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