JuliaPluto / HypertextLiteral.jl

Julia library for the string interpolation of HTML and SVG
https://juliapluto.github.io/HypertextLiteral.jl/stable/
ISC License
64 stars 10 forks source link

Problems with the String output of pretty_table from PrettyTables.jl in Pluto cell #11

Closed disberd closed 3 years ago

disberd commented 3 years ago

I am not sure whether this issue should be filed here on on PrettyTables, but basically using the @htl macro on the String output from the pretty_table functions doesn't correctly renders the HTML in a pluto cell, but simply puts the HTML code inside the text of the pluto output.

The same string is correctly parsed by the HTML() function.

Here is an example:

image

I don't know whether this is expected or if it is indeed an issue that was not known.

clarkevans commented 3 years ago

My guess is that the PrettyTables output isn't implementing Base.show(io::IO, m::MIME"text/html", ... ), hence, HypertextLiteral doesn't know that the output is intended to be pre-rendered HTML.

julia> x = pretty_table(String, rand(2,2); backend=:html, standalone=false);

julia> display("text/html", x)
ERROR: MethodError: no method matching show(::Base.TTY, ::MIME{Symbol("text/html")}, ::String)

julia> typeof(x)
String

julia> x = pretty_table(HTML, rand(2,2); backend=:html, standalone=false);
ERROR: The type UnionAll is not supported.

So, pretty_table should probably be returning HTML object in this case. Since it's returning a String object we can't even override it on our end to patch things. In this case, HypertextLiteral is behaving as it should, fully escaping the String object.

clarkevans commented 3 years ago

So, I opened a ticket over in PrettyTables with this detail. I hope this helps.

clarkevans commented 3 years ago

Oh. I see why you've thought this way. Hmm. I hadn't thought of @htl used in this manner. Let me investigate further. Even so, PrettyTables should still be updated to return an object that is showable("text/html", ...).

disberd commented 3 years ago

@clarkevans, thanks for the prompt reply and overall for the package that seems awesome. I indeed filed the issue here while probably it is more relevant to the other package.

At default PrettyTables prints the html directly to the stdout and I didn't find a way to provide a custom io rathern than stdout. In some discussion I was seeing the suggestion of using the form I put above HTML(pretty_table(String,...)) for correctly displaying on Pluto and given that this package seems to be just better in every aspect with respect to the standard HTML function I just tried to translate it directly.

I was in a bit of rush to get results so I didn't go too much into the documentation details of both packages so I will do that now that I have some more time and hopefully give more context.

clarkevans commented 3 years ago

Thank you for reporting this. I can see how this was confusing, moreover, the behavior was incorrect. I was treating the argument as a callable expression returning value that should be escaped. What you have is a function that is returning a string. The macro actually needs a string literal, not a string value or a function returning a string. So, I've fixed this issue... by raising an error: "a string literal is required"

One option I considered (but rejected) is to evaluate the expression immediately, expr = __module__.eval(code), and then use the String result of that expression as a template. However, this creates the opportunity for unintended outcomes, as described by Mason Protter. I think if someone wishes to to dynamically build a template, they might use @htl_str() instead.

disberd commented 3 years ago

Thanks for the clarification.

In the end, in case someone has a similar issue, for my use case I realized I could achieve my functionality by writing:

s = pretty_table(String,args...;backend=:html,standalone=false)
eval(:(@htl $s))