Skip to content

Commit

Permalink
🐜 Fix #68 and fix #52
Browse files Browse the repository at this point in the history
  • Loading branch information
fonsp committed Apr 8, 2020
1 parent 10d2b23 commit c13ed8d
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 27 deletions.
65 changes: 62 additions & 3 deletions assets/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ document.addEventListener("DOMContentLoaded", () => {

if (errormessage) {
cellNode.querySelector("celloutput").innerHTML = "<pre><code></code></pre>"
cellNode.querySelector("celloutput").querySelector("code").innerText = errormessage
cellNode.querySelector("celloutput").querySelector("code").innerHTML = rewrittenError(errormessage)
cellNode.classList.add("error")
} else {
cellNode.classList.remove("error")
Expand Down Expand Up @@ -300,11 +300,11 @@ document.addEventListener("DOMContentLoaded", () => {

/* REQUEST FUNCTIONS FOR REMOTE CHANGES */

function requestChangeRemoteCell(uuid) {
function requestChangeRemoteCell(uuid, createPromise=false) {
window.localCells[uuid].classList.add("running")
newCode = window.codeMirrors[uuid].getValue()

client.send("changecell", { code: newCode }, uuid)
return client.send("changecell", { code: newCode }, uuid, createPromise)
}

function requestRunAllRemoteCells() {
Expand Down Expand Up @@ -595,7 +595,66 @@ document.addEventListener("DOMContentLoaded", () => {
})
}

/* ERROR HINTS */

const hint1 = "Combine all definitions into a single reactive cell using a `begin ... end` block."
const hint2 = "Wrap all code in a `begin ... end` block."

const hint1_interactive = "<a href=\"#\" onclick=\"errorHint1(event)\">" + hint1 + "</a>"
const hint2_interactive = "<a href=\"#\" onclick=\"errorHint2(event)\">" + hint2 + "</a>"

function getOthers(e){
const cellNode = e.target.parentElement.parentElement.parentElement.parentElement
const myError = e.target.parentElement.innerHTML
var others = []
for (var uuid in window.localCells) {
const c = window.localCells[uuid]
if(c.classList.contains("error") && c.querySelector("celloutput>pre>code").innerHTML == myError) {
others.push(uuid)
}
}
return others
}
window.errorHint1 = (e) => {
combineCells(getOthers(e))
}

window.errorHint2 = (e) => {
const cellNode = e.target.parentElement.parentElement.parentElement.parentElement
wrapInBlock(window.codeMirrors[cellNode.id], "begin")
requestChangeRemoteCell(cellNode.id)
}

function rewrittenError(old_raw) {
return old_raw//.replace(hint1, hint1_interactive)
.replace(hint2, hint2_interactive)
}

function combineCells(uuids){
selectedCellNodes = uuids.map(u => localCells[u])
selectedCodeMirrors = uuids.map(u => codeMirrors[u])

client.sendreceive("addcell", {
index: indexOfLocalCell(selectedCellNodes[0]),
}).then(update => {
const combinedCode = selectedCodeMirrors.map(cm => cm.getValue()).join("\n\n")
createLocalCell(update.message.index, update.cellID, combinedCode)
wrapInBlock(codeMirrors[update.cellID], "begin")

requestChangeRemoteCell(update.cellID, true).then( u=> {
uuids.map(requestDeleteRemoteCell)
})

})
}

function wrapInBlock(cm, block="begin") {
const oldVal = cm.getValue()
cm.setValue(block + "\n\t" + oldVal.replace(/\n/g, "\n\t") + '\n' + "end")
}

/* MORE SHORTKEYS */

document.addEventListener("keydown", (e) => {
switch (e.keyCode) {
case 191: // ? or /
Expand Down
15 changes: 13 additions & 2 deletions src/react/Errors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,29 @@ function MultipleDefinitionsError(cell::Cell, all_definers)
MultipleDefinitionsError
end

struct MultipleExpressionsError <: ReactivityError
end

# Also update identically name variables in `editor.js`.
hint1 = "Combine all definitions into a single reactive cell using a `begin ... end` block."
hint2 = "Wrap all code in a `begin ... end` block."

# TODO: handle case when cells are in cycle, but variables aren't
function showerror(io::IO, cre::CyclicReferenceError)
print(io, "Cyclic references among $(join(cre.syms, ", ", " and ")).")
print(io, "Cyclic references among $(join(cre.syms, ", ", " and ")).\n$hint1")
end

function showerror(io::IO, mde::MultipleDefinitionsError)
print(io, "Multiple definitions for $(join(mde.syms, ", ", " and ")).\nCombine all definitions into a single reactive cell using a `begin` ... `end` block.") # TODO: hint about mutable globals
print(io, "Multiple definitions for $(join(mde.syms, ", ", " and ")).\n$hint1") # TODO: hint about mutable globals
end

function showerror(io::IO, mee::MultipleExpressionsError)
print(io, "Multiple expressions in one cell.\n$hint2")
end

"Send `error` to the frontend without backtrace. Runtime errors are handled by `WorkspaceManager.eval_fetch_in_workspace` - this function is for Reactivity errors."
function relay_reactivity_error!(cell::Cell, error::Exception)
cell.output_repr = nothing
cell.runtime = missing
cell.error_repr, cell.repr_mime = PlutoFormatter.format_output(error)
end
7 changes: 3 additions & 4 deletions src/react/Run.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ function run_reactive!(notebook::Notebook, cells::Array{Cell, 1})
to_delete_vars = union(to_delete_vars, (errable.symstate.assignments for errable in new_errable)...)
to_delete_funcs = union(to_delete_funcs, (Set(keys(errable.symstate.funcdefs)) for errable in new_errable)...)

workspace = WorkspaceManager.get_workspace(notebook)
WorkspaceManager.delete_vars(workspace, to_delete_vars)
to_reimport = union(Set{Expr}(), map(c -> c.module_usings, setdiff(notebook.cells, to_run))...)
WorkspaceManager.delete_vars(notebook, to_delete_vars, to_reimport)

local any_interrupted = false
for cell in to_run
Expand Down Expand Up @@ -78,9 +78,8 @@ run_reactive_async!(notebook::Notebook, cell::Cell) = run_reactive_async!(notebo

"Run a single cell non-reactively, return whether the run was Interrupted."
function run_single!(notebook::Notebook, cell::Cell)::Bool
starttime = time_ns()
run = WorkspaceManager.eval_fetch_in_workspace(notebook, cell.parsedcode)
cell.runtime = time_ns() - starttime
cell.runtime = run.runtime

if run.errored
cell.output_repr = nothing
Expand Down
59 changes: 42 additions & 17 deletions src/react/WorkspaceManager.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,43 @@ function get_workspace(notebook::Notebook)::Workspace
end

"Evaluate expression inside the workspace - output is fetched and formatted, errors are caught and formatted. Returns formatted output and error flags."
function eval_fetch_in_workspace(notebook::Notebook, expr)::NamedTuple{(:output_formatted, :errored, :interrupted),Tuple{Tuple{String,MIME},Bool,Bool}}
function eval_fetch_in_workspace(notebook::Notebook, expr)::NamedTuple{(:output_formatted, :errored, :interrupted, :runtime),Tuple{Tuple{String,MIME},Bool,Bool,Union{UInt64, Missing}}}
eval_fetch_in_workspace(get_workspace(notebook), expr)
end

function eval_fetch_in_workspace(workspace::Workspace, expr)::NamedTuple{(:output_formatted, :errored, :interrupted),Tuple{Tuple{String,MIME},Bool,Bool}}
function eval_fetch_in_workspace(workspace::Workspace, expr)::NamedTuple{(:output_formatted, :errored, :interrupted, :runtime),Tuple{Tuple{String,MIME},Bool,Bool,Union{UInt64, Missing}}}
# nasty fix:
if expr isa Expr && expr.head == :toplevel
expr.head = :block
end

timed_expr = if expr isa Expr && expr.head === :module
# Modules can only be defined at top level, so we need another eval. (i think)
# This adds extra runtime, so we don't time.
quote
(eval($(expr |> QuoteNode)), missing)
end
else
# For normal expressions:
# similar to @time source code
quote
local elapsed_ns = time_ns()
local result = ( $(expr) )
elapsed_ns = time_ns() - elapsed_ns
(result, elapsed_ns)
end
end

# We wrap the expression in a try-catch block, because we want to capture and format the exception on the worker itself.
wrapped = :(ans = try
# We want to eval `expr` in the global scope, try introduced a local scope.
Core.eval($(workspace.module_name), $(expr |> QuoteNode))
catch ex
bt = stacktrace(catch_backtrace())
CapturedException(ex, bt)
end)
wrapped = quote
ans, runtime = try
# We eval `expr` in the global scope of the workspace module:
Core.eval($(workspace.module_name), $(timed_expr |> QuoteNode))
catch ex
bt = stacktrace(catch_backtrace())
CapturedException(ex, bt), missing
end
end

# run the code πŸƒβ€β™€οΈ
# we use [pid] instead of pid to prevent fetching output
Expand Down Expand Up @@ -170,14 +194,9 @@ function eval_fetch_in_workspace(workspace::Workspace, expr)::NamedTuple{(:outpu

# instead of fetching the output value (which might not make sense in our context, since the user can define structs, types, functions, etc), we format the cell output on the worker, and fetch the formatted output.
# This also means that very big objects are not duplicated in RAM.
fetcher = :((output_formatted = PlutoFormatter.format_output(ans), errored = isa(ans, CapturedException), interrupted = false))
fetcher = :((output_formatted = PlutoFormatter.format_output(ans), errored = isa(ans, CapturedException), interrupted = false, runtime = runtime))

try
result = Distributed.remotecall_eval(Main, workspace.workspace_pid, fetcher)
return result
catch ex
rethrow(ex)
end
return Distributed.remotecall_eval(Main, workspace.workspace_pid, fetcher)
end

"Evaluate expression inside the workspace - output is not fetched, errors are rethrown. For internal use."
Expand Down Expand Up @@ -374,15 +393,21 @@ function move_vars(workspace::Workspace, old_workspace_name::Symbol, new_workspa
end
end
end


end

Distributed.remotecall_eval(Main, [workspace.workspace_pid], deleter)

for expr in module_imports_to_move
Distributed.remotecall_eval(Main, [workspace.workspace_pid], :(Core.eval($(new_workspace_name), $(expr |> QuoteNode))))
end
end


"Fake deleting variables by adding them to the workspace's blacklist."
function delete_vars(notebook::Notebook, to_delete::Set{Symbol}, module_imports_to_move::Set{Expr}=Set{Expr}())
delete_vars(get_workspace(notebook), to_delete)
delete_vars(get_workspace(notebook), to_delete, module_imports_to_move)
end

function delete_vars(workspace::Workspace, to_delete::Set{Symbol}, module_imports_to_move::Set{Expr}=Set{Expr}())
Expand Down
2 changes: 1 addition & 1 deletion src/webserver/WebServer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ function run(port = 1234, launchbrowser = false)
end
catch e
if isa(e, InterruptException)
println("\nClosing Pluto... Restart Julia for a fresh session. \n\nHave a nice day! 🎈")
println("\n\nClosing Pluto... Restart Julia for a fresh session. \n\nHave a nice day! 🎈")
close(serversocket)
for (uuid, ws) in WorkspaceManager.workspaces
WorkspaceManager.unmake_workspace(ws)
Expand Down
1 change: 1 addition & 0 deletions test/ExploreExpression.jl
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ end
@test testee(:((a[1], b.r) = (1, 2)), [], [], [], [])
@test testee(:((k = 2; 123)), [], [:k], [], [])
@test testee(:((a = 1; b = a + 1)), [:+], [:a, :b], [:+], [])
@test testee(Meta.parse("a = 1; b = a + 1"), [:+], [:a, :b], [:+], [])
@test testee(:((a = b = 1)), [], [:a, :b], [], [])
@test testee(:(let k = 2; 123 end), [], [], [], [])
@test testee(:(let k() = 2 end), [], [], [], [])
Expand Down
14 changes: 14 additions & 0 deletions test/React.jl
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ WorkspaceManager.set_default_distributed(false)
import Base: show
show(io::IO, x::Tuple) = write(io, \"🐟\")
end"),

Cell("using Dates"),
Cell("year(DateTime(28))")
])
fakeclient.connected_notebook = notebook

Expand Down Expand Up @@ -388,7 +391,18 @@ WorkspaceManager.set_default_distributed(false)
run_reactive!(notebook, notebook.cells[26])
run_reactive!(notebook, notebook.cells[25])
@test_broken notebook.cells[25].output_repr == "(25, :fish)"
end

@testset "Using external libraries" begin
run_reactive!(notebook, notebook.cells[27:28])
@test notebook.cells[27].error_repr == nothing
@test notebook.cells[28].output_repr == "28"
run_reactive!(notebook, notebook.cells[28])
@test notebook.cells[28].output_repr == "28"

notebook.cells[27].code = ""
run_reactive!(notebook, notebook.cells[27:28])
@test occursin("UndefVarError", notebook.cells[28].error_repr)
end
WorkspaceManager.unmake_workspace(notebook)

Expand Down

0 comments on commit c13ed8d

Please sign in to comment.