Skip to content

Commit

Permalink
Implement is_expandable extension interface
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelHatherly committed May 22, 2024
1 parent 224d278 commit a9b5cc0
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 16 deletions.
22 changes: 19 additions & 3 deletions src/QuartoNotebookWorker/src/render.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,16 @@ function render(
line::Integer,
cell_options::AbstractDict = Dict{String,Any}(),
)
return Base.@invokelatest(
# This records whether the outermost cell is an expandable cell, which we
# then return to the server so that it can decide whether to treat the cell
# results it gets back as an expansion or not. We can't decide this
# statically since expansion depends on whether the runtime type of the cell
# output is `is_expandable` or not. Recursive calls to `_render_thunk` don't
# matter to the server, it's just the outermost cell that matters.
is_expand_ref = Ref(false)
result = Base.@invokelatest(

Check warning on line 14 in src/QuartoNotebookWorker/src/render.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/src/render.jl#L13-L14

Added lines #L13 - L14 were not covered by tests
collect(
_render_thunk(code, cell_options) do
_render_thunk(code, cell_options, is_expand_ref) do
Base.@invokelatest include_str(
NotebookState.notebook_module(),
code;
Expand All @@ -16,8 +23,11 @@ function render(
end,
)
)
return (result, is_expand_ref[])

Check warning on line 26 in src/QuartoNotebookWorker/src/render.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/src/render.jl#L26

Added line #L26 was not covered by tests
end

is_expandable(@nospecialize(value)) = false

Check warning on line 29 in src/QuartoNotebookWorker/src/render.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/src/render.jl#L29

Added line #L29 was not covered by tests

# Recursively render cell thunks. This might be an `include_str` call,
# which is the starting point for a source cell, or it may be a
# user-provided thunk that comes from a source cell with `expand` set
Expand All @@ -26,9 +36,15 @@ function _render_thunk(
thunk::Base.Callable,
code::AbstractString,
cell_options::AbstractDict = Dict{String,Any}(),
is_expand_ref::Ref{Bool} = Ref(false),
)
captured, display_results = with_inline_display(thunk, cell_options)
if get(cell_options, "expand", false) === true

is_expand = is_expandable(captured.value) || get(cell_options, "expand", false) === true

Check warning on line 43 in src/QuartoNotebookWorker/src/render.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/src/render.jl#L43

Added line #L43 was not covered by tests
# Track in this side-channel whether the cell is to be expanded or not.
is_expand_ref[] = is_expand

Check warning on line 45 in src/QuartoNotebookWorker/src/render.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/src/render.jl#L45

Added line #L45 was not covered by tests

if is_expand

Check warning on line 47 in src/QuartoNotebookWorker/src/render.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/src/render.jl#L47

Added line #L47 was not covered by tests
if captured.error
return ((;
code = "", # an expanded cell that errored can't have returned code
Expand Down
41 changes: 29 additions & 12 deletions src/server.jl
Original file line number Diff line number Diff line change
Expand Up @@ -612,12 +612,9 @@ function evaluate_raw_cells!(

@maybe_progress showprogress "$header" for (nth, chunk) in enumerate(chunks)
if chunk.type === :code
expand_cell = get(chunk.cell_options, "expand", false) === true
# When we're not evaluating the code, or when there is an `expand`
# cell output then we immediately splice in the cell code. The
# results of evaluating an `expand` cell are added later on and are
# not considered direct outputs of this cell.
if !chunk.evaluate || expand_cell
if !chunk.evaluate
# Cells that are not evaluated are not executed, but they are
# still included in the notebook.
push!(
cells,
(;
Expand All @@ -626,12 +623,10 @@ function evaluate_raw_cells!(
metadata = (;),
source = process_cell_source(chunk.source),
outputs = [],
execution_count = chunk.evaluate ? 1 : 0,
execution_count = 0,
),
)
end

if chunk.evaluate
else
chunk_callback(ith_chunk_to_evaluate, chunks_to_evaluate, chunk)
ith_chunk_to_evaluate += 1

Expand All @@ -646,7 +641,26 @@ function evaluate_raw_cells!(
$(chunk.cell_options),
))

for (mth, remote) in enumerate(Malt.remote_eval_fetch(f.worker, expr))
worker_results, expand_cell = Malt.remote_eval_fetch(f.worker, expr)

# When the result of the cell evaluation is a cell expansion
# then we insert the original cell contents before the expanded
# cells as a mock cell similar to if it has `eval: false` set.
if expand_cell
push!(
cells,
(;
id = string(nth),
cell_type = chunk.type,
metadata = (;),
source = process_cell_source(chunk.source),
outputs = [],
execution_count = 1,
),
)
end

for (mth, remote) in enumerate(worker_results)
outputs = []
processed = process_results(remote.results)

Expand Down Expand Up @@ -800,7 +814,10 @@ function evaluate_raw_cells!(
# There should only ever be a single result from an
# inline evaluation since you can't pass cell
# options and so `expand` will always be `false`.
remote = only(Malt.remote_eval_fetch(f.worker, expr))
worker_results, expand_cell =
Malt.remote_eval_fetch(f.worker, expr)
expand_cell && error("inline code cells cannot be expanded")
remote = only(worker_results)
if !isnothing(remote.error)
# file location is not straightforward to determine with inline literals, but just printing the (presumably short)
# code back instead of a location should be quite helpful
Expand Down
17 changes: 17 additions & 0 deletions test/examples/cell_expansion.qmd
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,20 @@ cells(q::QuartoCell) = (q,)
QuartoCell(() -> 123, Dict(), "")
```

```{julia}
import QuartoNotebookWorker
# Actual usage of this feature requires the `QuartoNotebookWorker` module to be
# weakdep of the package that would like to use it and define this method in an
# package extension module. You cannot add `QuartoNotebookWorker` as a real
# dependency since it is not, and cannot, be registered.
QuartoNotebookWorker.is_expandable(::QuartoCell) = true
```

Extending the `is_expandable` method to return `true` for the `QuartoCell` type
should result in the equivalent of `expand: true` being set for the cell.

```{julia}
QuartoCell(() -> 123, Dict(), "")
```
5 changes: 4 additions & 1 deletion test/testsets/cell_expansion.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
include("../utilities/prelude.jl")

test_example(joinpath(@__DIR__, "../examples/cell_expansion.qmd")) do json
@test length(json["cells"]) == 13
@test length(json["cells"]) == 18

cell = json["cells"][1]
@test cell["cell_type"] == "markdown"
Expand Down Expand Up @@ -80,6 +80,9 @@ test_example(joinpath(@__DIR__, "../examples/cell_expansion.qmd")) do json

cell = json["cells"][13]
@test cell["outputs"][1]["data"]["text/plain"] == "123"

cell = json["cells"][18]
@test cell["outputs"][1]["data"]["text/plain"] == "123"
end

test_example(joinpath(@__DIR__, "../examples/cell_expansion_errors.qmd")) do json
Expand Down

0 comments on commit a9b5cc0

Please sign in to comment.