JuliaLang / IJulia.jl

Julia kernel for Jupyter
MIT License
2.78k stars 409 forks source link

Broken display of `AbstractString`s when preferred MIME type `istextmime` #1113

Closed jakobjpeters closed 2 months ago

jakobjpeters commented 3 months ago

Description

Users implementing show methods for an AbstractString will have a broken display if the preferred MIME type istextmime. A motivating example is in Typstry.jl, which implements show for "image/png" and "image/svg+xml". Both of these methods work in Pluto.jl, but only "image/png" works in IJulia.jl. This is because "image/svg+xml" is preferred over "image/png", but this format assumes that an AbstractString is "raw data". Instead of dispatching to show, it is returned as a String.

I don't really understand this assumption, why is it being used? The only thing I have found so far is the same assumption in repr and presume that's where it came from. A notable difference is that users are able to implement repr to patch in a fix.

Possible Solutions

1. The output of versioninfo()

IJulia v0.24.2

julia> versioninfo()
Julia Version 1.10.3
Commit 0b4590a5507 (2024-04-30 10:59 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, tigerlake)
Threads: 8 default, 0 interactive, 4 GC (on 8 virtual cores)
Environment:
  JULIA_EDITOR = hx

2. How you installed Julia

juliaup

3. A minimal working example (MWE), also known as a minimum reproducible example

julia> using IJulia

julia> struct S <: AbstractString end

julia> Base.ncodeunits(::S) = 0;

julia> Base.iterate(::S) = nothing;

julia> Base.show(io::IO, ::MIME"image/svg+xml", ::S) = print(io, "s");

julia> S()
""

julia> show(stdout, "image/svg+xml", S())
s
julia> IJulia.display_dict(S())
Dict{String, Union{String, JSON.Writer.JSONText}} with 2 entries:
  "text/plain"    => "\"\""
  "image/svg+xml" => ""
stevengj commented 3 months ago

Why would an AbstractString subtype have a method to display as image/svg+xml? That doesn't sound like a string?

The reason for this behavior is that it allows you to do things like display("image/svg+xml", "...raw SVG text as an ordinary string...") and it will show an SVG image from the given string data interpreted as XML. (But we can probably avoid that behavior for the display_dict if needed.)

jakobjpeters commented 3 months ago

My particular use case is for a TypstString in Typstry.jl, which represents source text for the Typst typsetting system. This text can be compiled to PDF, PNG, and SVG via Typst_jll.jl, so there's a direct correspondence between a TypstString and an SVG. Although the underlying implementation differs, I'd like typst"$ 1 / x $" to be rendered in notebooks in a similar manner to L"$ \frac{1}{x} $".

Thank you for the clarification! Does display_dict determine resulting output options? This comment seems to indicate so. If so, then is the solution to keep the current behavior for calls to limitstringmime except from within display_dict?

stevengj commented 3 months ago

Yes, it would probably suffice to change the definition of limitstringmime to:

function limitstringmime(mime::MIME, x, forcetext=false, mayberawtext=true)

change this line to

if israwtext(mime, x) && mayberawtext

and change this line to:

display_mimestring(m::MIME, x) = (m, limitstringmime(m, x, false, false))
stevengj commented 3 months ago

Alternatively, an even simpler and probably better solution would be to change this line to

israwtext(m::MIME, x::AbstractString) = !showable(m, x)

so that we only treat x as "raw" MIME data if it doesn't have it's own show method for that MIME type.