add msgpack_type method Expr and related

[disberd]

This PR should fix #2652

[github-actions]

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/disberd/Pluto.jl", rev="msgpack_expr")
julia> using Pluto

[pankgeorg]

Can you add a test? (I'm just curious how it gets serialized 😅)

[disberd]

@pankgeorg serialization is just a Dict:

julia> MsgPack.pack(:(LOL = [1, "string", :symbol])) |> MsgPack.unpack
Dict{Any, Any} with 2 entries:
  "head" => "="
  "args" => Any["LOL", Dict{Any, Any}("head"=>"vect", "args"=>Any[0x01, "string", Dict{Any, Any}("value"=>"symbol")])]

The problem is in de-serialization. It is not straightforward to reconstruct the original Expr like this.

But I am thinking, isnt' MsgPack just used to send data to the front-end? Since the test in

Pluto.jl/test/Configuration.jl

Lines 128 to 133 in 27b23a4

	@testset "disable mimetype via workspace_custom_startup_expr" begin
	🍭 = ServerSession()
	🍭.options.evaluation.workspace_use_distributed = true
	🍭.options.evaluation.workspace_custom_startup_expr = quote
	PlutoRunner.is_mime_enabled(m::MIME"application/vnd.pluto.tree+object") = false
	end

already shows that this kwarg is correctly executed in the notebook workspace, maybe here is the problem is that this kwarg at least should not be packed and sent via MsgPack at all to the HTTP server?

Adding a MsgPack.pack method seems to just avoid the error but sending this Expr via MsgPack should probably be avoided in the first place?

[Pangoraw]

I think the msgpack serialization happens here when sending server config to the client:

Pluto.jl/src/webserver/Dynamic.jl

Line 397 in 871517c

:options => 🙋.session.options,

Maybe it is simpler to send the config option as a string or simply drop it from the config when sending the config to the client instead of a serialized Expr?

[fonsp]

I agree with Paul!

Actually I think the config workspace_custom_startup_expr should be changed to accept a String, which we parse ourselves. I think making that change would not be breaking since this setting is currently unusable.

[Pangoraw]

I think making that change would not be breaking since this setting is currently unusable.

@rikhuijzer is using it in PlutoStaticHTML, without client connections it works:

https://github.com/rikhuijzer/PlutoStaticHTML.jl/blob/234b907a1aecea1071146b5126594382a33efdf2/src/build.jl#L369-L373

But a string would also make it clear that you cannot embed arbitrary code with packages not loaded on the notebook like:

workspace_custom_startup_expr = quote
   MyPackage.do_something() # <- will not de-serialize on the notebook process
end

[fonsp]

A string is also necessary when using a TOML config like in PSS: https://github.com/JuliaPluto/PlutoSliderServer.jl#2-plutodeploymenttoml

Lemme see if I can do some hack with overloading Base.set! to support both...

[fonsp]

#2654

[rikhuijzer]

I think making that change would not be breaking since this setting is currently unusable.

@rikhuijzer is using it in PlutoStaticHTML, without client connections it works:

https://github.com/rikhuijzer/PlutoStaticHTML.jl/blob/234b907a1aecea1071146b5126594382a33efdf2/src/build.jl#L369-L373

But a string would also make it clear that you cannot embed arbitrary code with packages not loaded on the notebook like:

workspace_custom_startup_expr = quote
   MyPackage.do_something() # <- will not de-serialize on the notebook process
end

@Pangoraw No worries about breaking PlutoStaticHTML 😄 . As long as another option remains for handling the situation then it's fine. PlutoStaticHTML is pinned to an exact Pluto version so is very robust to breaking changes.