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

Fix: Don't drop disabled cells #2677

Merged
merged 3 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/analysis/Topology.jl
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,18 @@ function set_unresolved(topology::NotebookTopology, unresolved_cells::Vector{Cel
end


function Base.setdiff(topology::NotebookTopology, cells::Vector{Cell})
"""
exclude_roots(topology::NotebookTopology, roots_to_exclude)::NotebookTopology

Returns a new topology as if `topology` was created with all code for `roots_to_exclude`
being empty, preserving disabled cells and cell order.
"""
function exclude_roots(topology::NotebookTopology, cells::Vector{Cell})
NotebookTopology(
nodes=setdiffkeys(topology.nodes, cells),
codes=setdiffkeys(topology.codes, cells),
unresolved_cells=ImmutableSet{Cell}(setdiff(topology.unresolved_cells.c, cells); skip_copy=true),
cell_order=ImmutableVector{Cell}(setdiff(topology.cell_order.c, cells); skip_copy=true),
disabled_cells=ImmutableSet{Cell}(setdiff(topology.disabled_cells.c, cells); skip_copy=true),
cell_order=topology.cell_order,
disabled_cells=topology.disabled_cells,
)
end
4 changes: 2 additions & 2 deletions src/evaluation/Run.jl
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ function run_reactive_core!(
cell.queued = false
cell.depends_on_disabled_cells = true
end
new_topology = setdiff(new_topology, indirectly_deactivated)

new_topology = exclude_roots(new_topology, indirectly_deactivated)

# save the old topological order - we'll delete variables assigned from its
# and re-evalutate its cells unless the cells have already run previously in the reactive run
Expand Down
12 changes: 9 additions & 3 deletions src/notebook/saving and loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,24 @@ function save_notebook(io::IO, notebook::Notebook)
println(io)

cells_ordered = collect(topological_order(notebook))

# TODO: add test for this case
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add a comment here to explain when this can happen?

if length(cells_ordered) != length(notebook.cells)
cells = notebook.cells
updated_topo = updated_topology(notebook.topology, notebook, cells)
cells_ordered = collect(topological_order(updated_topo, cells))
end

for c in cells_ordered
println(io, _cell_id_delimiter, string(c.cell_id))

let metadata_toml = strip(sprint(TOML.print, get_metadata_no_default(c)))
if metadata_toml != ""
for line in split(metadata_toml, "\n")
println(io, _cell_metadata_prefix, line)
end
end
end

if must_be_commented_in_file(c)
print(io, _disabled_prefix)
print(io, replace(c.code, _cell_id_delimiter => "# "))
Expand Down
43 changes: 43 additions & 0 deletions test/cell_disabling.jl
Original file line number Diff line number Diff line change
Expand Up @@ -345,3 +345,46 @@ end

WorkspaceManager.unmake_workspace((🍭, notebook))
end

@testset "Disabled cells should stay in the topology (#2676)" begin
🍭 = ServerSession()
notebook = Notebook(Cell.([
"using Dates",
"b = 2; December",
"b",
]))

disabled_cell = notebook.cells[end]
Pluto.set_disabled(disabled_cell, true)
@test is_disabled(disabled_cell)

old_topo = notebook.topology
@test count(Pluto.is_disabled, notebook.cells) == 1
order = update_run!(🍭, notebook, notebook.cells)

# Disabled
@test length(order.input_topology.disabled_cells) == 1
@test disabled_cell ∈ order.input_topology.disabled_cells
runned_cells = collect(order)
@test length(runned_cells) == 2
@test disabled_cell ∉ runned_cells

topo = notebook.topology
@test old_topo !== topo # topology was updated

order = Pluto.topological_order(notebook)

@test length(order.input_topology.disabled_cells) == 1
@test disabled_cell ∈ order.input_topology.disabled_cells
saved_cells = collect(order)
@test length(saved_cells) == length(notebook.cells)
@test issetequal(saved_cells, notebook.cells)

io = IOBuffer()
Pluto.save_notebook(io, notebook)
seekstart(io)
notebook2 = Pluto.load_notebook_nobackup(io, "mynotebook.jl")
@test length(notebook2.cells) == length(notebook.cells)

WorkspaceManager.unmake_workspace((🍭, notebook))
end