Restore code markers and doctest indicators on refresh (#1949)

This commit is contained in:
Jonatan Kłosko 2023-05-31 15:52:08 +02:00 committed by GitHub
parent f507f67488
commit 04efa5932f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 282 additions and 144 deletions

View file

@ -205,4 +205,5 @@ Also some spacing adjustments.
padding-bottom: 6px;
position: absolute;
width: 100%;
overflow-y: auto;
}

View file

@ -253,6 +253,11 @@ const Cell = {
liveEditor.updateDoctest(doctestReport);
}
);
this.handleEvent(`erase_outputs`, () => {
liveEditor.setCodeMarkers([]);
liveEditor.clearDoctests();
});
}
}
},

View file

@ -1,5 +1,5 @@
import LiveEditor from "./cell_editor/live_editor";
import { getAttributeOrThrow } from "../lib/attribute";
import { getAttributeOrThrow, parseBoolean } from "../lib/attribute";
const CellEditor = {
mounted() {
@ -7,7 +7,7 @@ const CellEditor = {
this.handleEvent(
`cell_editor_init:${this.props.cellId}:${this.props.tag}`,
({ source_view, language, intellisense, read_only }) => {
({ source, revision, doctest_reports, code_markers }) => {
const editorContainer = this.el.querySelector(
`[data-el-editor-container]`
);
@ -20,11 +20,13 @@ const CellEditor = {
editorEl,
this.props.cellId,
this.props.tag,
source_view.source,
source_view.revision,
language,
intellisense,
read_only
source,
revision,
this.props.language,
this.props.intellisense,
this.props.readOnly,
code_markers,
doctest_reports
);
this.liveEditor.onMount(() => {
@ -68,6 +70,13 @@ const CellEditor = {
return {
cellId: getAttributeOrThrow(this.el, "data-cell-id"),
tag: getAttributeOrThrow(this.el, "data-tag"),
language: getAttributeOrThrow(this.el, "data-language"),
intellisense: getAttributeOrThrow(
this.el,
"data-intellisense",
parseBoolean
),
readOnly: getAttributeOrThrow(this.el, "data-read-only", parseBoolean),
};
},
};

View file

@ -22,7 +22,9 @@ class LiveEditor {
revision,
language,
intellisense,
readOnly
readOnly,
codeMarkers,
doctestReports
) {
this.hook = hook;
this.container = container;
@ -38,6 +40,14 @@ class LiveEditor {
this._remoteUserByClientId = {};
this._doctestByLine = {};
this._initializeWidgets = () => {
this.setCodeMarkers(codeMarkers);
doctestReports.forEach((doctestReport) => {
this.updateDoctest(doctestReport);
});
};
const serverAdapter = new HookServerAdapter(hook, cellId, tag);
this.editorClient = new EditorClient(serverAdapter, revision);
@ -351,6 +361,9 @@ class LiveEditor {
this.editor._modelData.view._contentWidgets.overflowingContentWidgetsDomNode.domNode.appendChild(
commandPaletteNode
);
// Add the widgets that the editor was initialized with
this._initializeWidgets();
}
/**

View file

@ -80,10 +80,6 @@ class DetailsWidget {
const detailsHtml = details.join("\n");
const numberOfLines = details.length;
const marginWidth = this._editor
.getDomNode()
.querySelector(".margin-view-overlays").offsetWidth;
const fontSize = this._editor.getOption(
monaco.editor.EditorOption.fontSize
);
@ -99,7 +95,6 @@ class DetailsWidget {
"editor-theme-aware-ansi"
);
detailsNode.style.fontSize = `${fontSize}px`;
detailsNode.style.paddingLeft = `calc(${marginWidth}px + ${column}ch)`;
this._overlayWidget = {
getId: () => `livebook.doctest.overlay.${line}`,
@ -117,6 +112,12 @@ class DetailsWidget {
domNode: document.createElement("div"),
onDomNodeTop: (top) => {
detailsNode.style.top = `${top}px`;
const marginWidth = this._editor
.getDomNode()
.querySelector(".margin-view-overlays").offsetWidth;
detailsNode.style.paddingLeft = `calc(${marginWidth}px + ${column}ch)`;
},
onComputedHeight: (height) => {
detailsNode.style.height = `${height}px`;

View file

@ -547,16 +547,6 @@ defmodule Livebook.Session.Data do
end
end
def apply_operation(data, {:add_cell_doctest_report, _client_id, id, _doctest_report}) do
with {:ok, _cell, _} <- Notebook.fetch_cell_and_section(data.notebook, id) do
data
|> with_actions()
|> wrap_ok()
else
_ -> :error
end
end
def apply_operation(data, {:add_cell_evaluation_output, _client_id, id, output}) do
with {:ok, cell, _} <- Notebook.fetch_cell_and_section(data.notebook, id) do
data
@ -587,6 +577,17 @@ defmodule Livebook.Session.Data do
end
end
def apply_operation(data, {:add_cell_doctest_report, _client_id, id, doctest_report}) do
with {:ok, cell, _} <- Notebook.fetch_cell_and_section(data.notebook, id) do
data
|> with_actions()
|> add_cell_doctest_report(cell, doctest_report)
|> wrap_ok()
else
_ -> :error
end
end
def apply_operation(data, {:bind_input, _client_id, cell_id, input_id}) do
with {:ok, cell, _section} <- Notebook.fetch_cell_and_section(data.notebook, cell_id),
Cell.evaluable?(cell),
@ -1224,7 +1225,8 @@ defmodule Livebook.Session.Data do
identifiers_used: metadata.identifiers_used,
identifiers_defined: metadata.identifiers_defined,
bound_to_input_ids: eval_info.new_bound_to_input_ids,
evaluation_end: DateTime.utc_now()
evaluation_end: DateTime.utc_now(),
code_markers: metadata.code_markers
}
end)
|> update_cell_evaluation_snapshot(cell, section)
@ -1263,6 +1265,14 @@ defmodule Livebook.Session.Data do
)
end
defp add_cell_doctest_report(data_actions, cell, doctest_report) do
data_actions
|> update_cell_eval_info!(
cell.id,
&put_in(&1.doctest_reports[doctest_report.line], doctest_report)
)
end
defp maybe_connect_runtime({data, _} = data_actions, prev_data) do
if not Runtime.connected?(data.runtime) and not any_cell_queued?(prev_data) and
any_cell_queued?(data) do
@ -1413,7 +1423,9 @@ defmodule Livebook.Session.Data do
# This is a rough estimate, the exact time is measured in the
# evaluator itself
evaluation_start: DateTime.utc_now(),
evaluation_end: nil
evaluation_end: nil,
code_markers: [],
doctest_reports: %{}
}
end)
end)
@ -1609,7 +1621,7 @@ defmodule Livebook.Session.Data do
|> update_every_cell_info(fn
%{eval: _} = info ->
info = update_in(info.eval.outputs_batch_number, &(&1 + 1))
put_in(info.eval.validity, :fresh)
update_in(info.eval, &%{&1 | validity: :fresh, code_markers: [], doctest_reports: %{}})
info ->
info
@ -2025,6 +2037,8 @@ defmodule Livebook.Session.Data do
snapshot: nil,
evaluation_snapshot: nil,
data: nil,
code_markers: [],
doctest_reports: %{},
reevaluates_automatically: false
}
end

View file

@ -51,7 +51,10 @@ defmodule LivebookWeb.SessionLive do
end)
}
push_event(socket, "session_init", payload)
socket = push_event(socket, "session_init", payload)
cells = for {cell, _} <- Notebook.cells_with_section(data.notebook), do: cell
push_cell_editor_payloads(socket, data, cells)
else
socket
end
@ -1720,6 +1723,11 @@ defmodule LivebookWeb.SessionLive do
end
defp after_operation(socket, _prev_socket, {:insert_cell, client_id, _, _, _, cell_id, _attrs}) do
{:ok, cell, _section} =
Notebook.fetch_cell_and_section(socket.private.data.notebook, cell_id)
socket = push_cell_editor_payloads(socket, socket.private.data, [cell])
socket = prune_cell_sources(socket)
if client_id == socket.assigns.client_id do
@ -1747,6 +1755,11 @@ defmodule LivebookWeb.SessionLive do
end
defp after_operation(socket, _prev_socket, {:restore_cell, client_id, cell_id}) do
{:ok, cell, _section} =
Notebook.fetch_cell_and_section(socket.private.data.notebook, cell_id)
socket = push_cell_editor_payloads(socket, socket.private.data, [cell])
socket = prune_cell_sources(socket)
if client_id == socket.assigns.client_id do
@ -1795,14 +1808,7 @@ defmodule LivebookWeb.SessionLive do
_prev_socket,
{:add_cell_doctest_report, _client_id, cell_id, doctest_report}
) do
doctest_report =
Map.replace_lazy(doctest_report, :details, fn details ->
details
|> LivebookWeb.Helpers.ANSI.ansi_string_to_html_lines()
|> Enum.map(&Phoenix.HTML.safe_to_string/1)
end)
push_event(socket, "doctest_report:#{cell_id}", doctest_report)
push_event(socket, "doctest_report:#{cell_id}", doctest_report_payload(doctest_report))
end
defp after_operation(
@ -1813,6 +1819,10 @@ defmodule LivebookWeb.SessionLive do
prune_cell_sources(socket)
end
defp after_operation(socket, _prev_socket, {:erase_outputs, _client_id}) do
push_event(socket, "erase_outputs", %{})
end
defp after_operation(socket, prev_socket, {:set_deployed_app_slug, _client_id, slug}) do
prev_slug = prev_socket.private.data.deployed_app_slug
@ -2041,6 +2051,74 @@ defmodule LivebookWeb.SessionLive do
end
end
defp push_cell_editor_payloads(socket, data, cells) do
for cell <- cells,
{tag, payload} <- cell_editor_init_payloads(cell, data.cell_infos[cell.id]),
reduce: socket do
socket ->
push_event(socket, "cell_editor_init:#{cell.id}:#{tag}", payload)
end
end
defp cell_editor_init_payloads(%Cell.Code{} = cell, cell_info) do
[
primary: %{
source: cell.source,
revision: cell_info.sources.primary.revision,
code_markers: cell_info.eval.code_markers,
doctest_reports:
for {_, doctest_report} <- cell_info.eval.doctest_reports do
doctest_report_payload(doctest_report)
end
}
]
end
defp cell_editor_init_payloads(%Cell.Markdown{} = cell, cell_info) do
[
primary: %{
source: cell.source,
revision: cell_info.sources.primary.revision,
code_markers: [],
doctest_reports: []
}
]
end
defp cell_editor_init_payloads(%Cell.Smart{} = cell, cell_info) do
[
primary: %{
source: cell.source,
revision: cell_info.sources.primary.revision,
code_markers: cell_info.eval.code_markers,
doctest_reports:
for {_, doctest_report} <- cell_info.eval.doctest_reports do
doctest_report_payload(doctest_report)
end
}
] ++
if cell.editor do
[
secondary: %{
source: cell.editor.source,
revision: cell_info.sources.secondary.revision,
code_markers: [],
doctest_reports: []
}
]
else
[]
end
end
defp doctest_report_payload(doctest_report) do
Map.replace_lazy(doctest_report, :details, fn details ->
details
|> LivebookWeb.Helpers.ANSI.ansi_string_to_html_lines()
|> Enum.map(&Phoenix.HTML.safe_to_string/1)
end)
end
# Builds view-specific structure of data by cherry-picking
# only the relevant attributes.
# We then use `@data_view` in the templates and consequently
@ -2149,13 +2227,11 @@ defmodule LivebookWeb.SessionLive do
%{id: section.id, name: section.name}
end
defp cell_to_view(%Cell.Markdown{} = cell, data) do
info = data.cell_infos[cell.id]
defp cell_to_view(%Cell.Markdown{} = cell, _data) do
%{
id: cell.id,
type: :markdown,
source_view: source_view(cell.source, info.sources.primary)
empty: cell.source == ""
}
end
@ -2165,7 +2241,7 @@ defmodule LivebookWeb.SessionLive do
%{
id: cell.id,
type: :code,
source_view: source_view(cell.source, info.sources.primary),
empty: cell.source == "",
eval: eval_info_to_view(cell, info.eval, data),
reevaluate_automatically: cell.reevaluate_automatically
}
@ -2177,16 +2253,16 @@ defmodule LivebookWeb.SessionLive do
%{
id: cell.id,
type: :smart,
source_view: source_view(cell.source, info.sources.primary),
empty: cell.source == "",
eval: eval_info_to_view(cell, info.eval, data),
status: info.status,
js_view: cell.js_view,
editor:
cell.editor &&
%{
empty: cell.editor.souruce == "",
language: cell.editor.language,
placement: cell.editor.placement,
source_view: source_view(cell.editor.source, info.sources.secondary)
placement: cell.editor.placement
}
}
end
@ -2207,17 +2283,6 @@ defmodule LivebookWeb.SessionLive do
}
end
defp source_view(:__pruned__, _source_info) do
:__pruned__
end
defp source_view(source, source_info) do
%{
source: source,
revision: source_info.revision
}
end
defp input_values_for_cell(cell, data) do
input_ids =
for output <- cell.outputs,

View file

@ -16,7 +16,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do
data-evaluation-digest={get_in(@cell_view, [:eval, :evaluation_digest])}
data-eval-validity={get_in(@cell_view, [:eval, :validity])}
data-eval-errored={get_in(@cell_view, [:eval, :errored])}
data-js-empty={empty?(@cell_view.source_view)}
data-js-empty={@cell_view.empty}
data-smart-cell-js-view-ref={smart_cell_js_view_ref(@cell_view)}
data-allowed-uri-schemes={Enum.join(@allowed_uri_schemes, ",")}
>
@ -38,12 +38,10 @@ defmodule LivebookWeb.SessionLive.CellComponent do
</.cell_actions>
<.cell_body>
<div class="pb-4" data-el-editor-box>
<.live_component
module={LivebookWeb.SessionLive.CellEditorComponent}
id={"#{@cell_view.id}-primary"}
<.cell_editor
cell_id={@cell_view.id}
tag="primary"
source_view={@cell_view.source_view}
empty={@cell_view.empty}
language="markdown"
/>
</div>
@ -53,7 +51,7 @@ defmodule LivebookWeb.SessionLive.CellComponent do
id={"markdown-container-#{@cell_view.id}"}
phx-update="ignore"
>
<.content_skeleton empty={empty?(@cell_view.source_view)} />
<.content_skeleton empty={@cell_view.empty} />
</div>
</.cell_body>
"""
@ -83,12 +81,10 @@ defmodule LivebookWeb.SessionLive.CellComponent do
</.cell_actions>
<.cell_body>
<div class="relative">
<.live_component
module={LivebookWeb.SessionLive.CellEditorComponent}
id={"#{@cell_view.id}-primary"}
<.cell_editor
cell_id={@cell_view.id}
tag="primary"
source_view={@cell_view.source_view}
empty={@cell_view.empty}
language="elixir"
intellisense
/>
@ -132,12 +128,10 @@ defmodule LivebookWeb.SessionLive.CellComponent do
</div>
<div data-el-editor-box>
<div class="relative">
<.live_component
module={LivebookWeb.SessionLive.CellEditorComponent}
id={"#{@cell_view.id}-primary"}
<.cell_editor
cell_id={@cell_view.id}
tag="primary"
source_view={@cell_view.source_view}
empty={@cell_view.empty}
language="elixir"
intellisense
/>
@ -192,13 +186,11 @@ defmodule LivebookWeb.SessionLive.CellComponent do
session_id={@session_id}
client_id={@client_id}
/>
<.live_component
<.cell_editor
:if={@cell_view.editor}
module={LivebookWeb.SessionLive.CellEditorComponent}
id={"#{@cell_view.id}-secondary"}
cell_id={@cell_view.id}
tag="secondary"
source_view={@cell_view.editor.source_view}
empty={@cell_view.editor.empty}
language={@cell_view.editor.language}
rounded={@cell_view.editor.placement}
/>
@ -235,12 +227,10 @@ defmodule LivebookWeb.SessionLive.CellComponent do
</div>
<div data-el-editor-box>
<div class="relative">
<.live_component
module={LivebookWeb.SessionLive.CellEditorComponent}
id={"#{@cell_view.id}-primary"}
<.cell_editor
cell_id={@cell_view.id}
tag="primary"
source_view={@cell_view.source_view}
empty={@cell_view.empty}
language="elixir"
intellisense
read_only
@ -580,6 +570,39 @@ defmodule LivebookWeb.SessionLive.CellComponent do
"""
end
attr :cell_id, :string, required: true
attr :tag, :string, required: true
attr :empty, :boolean, required: true
attr :language, :string, required: true
attr :intellisense, :boolean, default: false
attr :read_only, :boolean, default: false
attr :rounded, :atom, default: :both
defp cell_editor(assigns) do
~H"""
<div
id={"cell-editor-#{@cell_id}-#{@tag}"}
phx-update="ignore"
phx-hook="CellEditor"
data-cell-id={@cell_id}
data-tag={@tag}
data-language={@language}
data-intellisense={to_string(@intellisense)}
data-read-only={to_string(@read_only)}
>
<div class={["py-3 bg-editor", rounded_class(@rounded)]} data-el-editor-container>
<div class="px-8" data-el-skeleton>
<.content_skeleton bg_class="bg-gray-500" empty={@empty} />
</div>
</div>
</div>
"""
end
defp rounded_class(:both), do: "rounded-lg"
defp rounded_class(:top), do: "rounded-t-lg"
defp rounded_class(:bottom), do: "rounded-b-lg"
defp evaluation_outputs(assigns) do
~H"""
<div
@ -601,9 +624,6 @@ defmodule LivebookWeb.SessionLive.CellComponent do
"""
end
defp empty?(%{source: ""} = _source_view), do: true
defp empty?(_source_view), do: false
defp cell_status(%{cell_view: %{eval: %{status: :evaluating}}} = assigns) do
~H"""
<.cell_status_indicator variant={:progressing} change_indicator={true}>

View file

@ -1,63 +0,0 @@
defmodule LivebookWeb.SessionLive.CellEditorComponent do
use LivebookWeb, :live_component
@impl true
def mount(socket) do
{:ok, assign(socket, initialized: false)}
end
@impl true
def update(assigns, socket) do
socket =
socket
|> assign(assigns)
|> assign_new(:intellisense, fn -> false end)
|> assign_new(:read_only, fn -> false end)
|> assign_new(:rounded, fn -> :both end)
socket =
if not connected?(socket) or socket.assigns.initialized do
socket
else
socket
|> push_event(
"cell_editor_init:#{socket.assigns.cell_id}:#{socket.assigns.tag}",
%{
source_view: socket.assigns.source_view,
language: socket.assigns.language,
intellisense: socket.assigns.intellisense,
read_only: socket.assigns.read_only
}
)
|> assign(initialized: true)
end
{:ok, socket}
end
@impl true
def render(assigns) do
~H"""
<div
id={"cell-editor-#{@id}"}
phx-update="ignore"
phx-hook="CellEditor"
data-cell-id={@cell_id}
data-tag={@tag}
>
<div class={["py-3 bg-editor", rounded_class(@rounded)]} data-el-editor-container>
<div class="px-8" data-el-skeleton>
<.content_skeleton bg_class="bg-gray-500" empty={empty?(@source_view)} />
</div>
</div>
</div>
"""
end
defp empty?(%{source: ""} = _source_view), do: true
defp empty?(_source_view), do: false
defp rounded_class(:both), do: "rounded-lg"
defp rounded_class(:top), do: "rounded-t-lg"
defp rounded_class(:bottom), do: "rounded-b-lg"
end

View file

@ -22,7 +22,8 @@ defmodule Livebook.Session.DataTest do
interrupted: interrupted,
evaluation_time_ms: 10,
identifiers_used: uses,
identifiers_defined: defines
identifiers_defined: defines,
code_markers: []
}
end
@ -2493,6 +2494,59 @@ defmodule Livebook.Session.DataTest do
end
end
describe "apply_operation/2 given :add_cell_doctest_report" do
test "returns an error given invalid cell id" do
data = Data.new()
operation = {:add_cell_doctest_report, @cid, "c1", %{status: :running, line: 5}}
assert :error = Data.apply_operation(data, operation)
end
test "adds doctest report to cell evaluation info" do
data =
data_after_operations!([
{:insert_section, @cid, 0, "s1"},
{:insert_cell, @cid, "s1", 0, :code, "c1", %{}},
{:set_runtime, @cid, connected_noop_runtime()},
evaluate_cells_operations(["setup"]),
{:queue_cells_evaluation, @cid, ["c1"]}
])
doctest_report = %{status: :running, line: 5}
operation = {:add_cell_doctest_report, @cid, "c1", doctest_report}
assert {:ok,
%{
cell_infos: %{
"c1" => %{eval: %{doctest_reports: %{5 => ^doctest_report}}}
}
}, []} = Data.apply_operation(data, operation)
end
test "updates doctest report by line" do
data =
data_after_operations!([
{:insert_section, @cid, 0, "s1"},
{:insert_cell, @cid, "s1", 0, :code, "c1", %{}},
{:set_runtime, @cid, connected_noop_runtime()},
evaluate_cells_operations(["setup"]),
{:queue_cells_evaluation, @cid, ["c1"]},
{:add_cell_doctest_report, @cid, "c1", %{status: :running, line: 5}}
])
doctest_report = %{status: :success, line: 5}
operation = {:add_cell_doctest_report, @cid, "c1", doctest_report}
assert {:ok,
%{
cell_infos: %{
"c1" => %{eval: %{doctest_reports: %{5 => ^doctest_report}}}
}
}, []} = Data.apply_operation(data, operation)
end
end
describe "apply_operation/2 given :bind_input" do
test "returns an error given invalid input cell id" do
data =
@ -3047,6 +3101,24 @@ defmodule Livebook.Session.DataTest do
}
}, _actions} = Data.apply_operation(data, operation)
end
test "removes doctest reports" do
data =
data_after_operations!([
{:insert_section, @cid, 0, "s1"},
{:insert_cell, @cid, "s1", 0, :code, "c1", %{}},
{:set_runtime, @cid, connected_noop_runtime()},
evaluate_cells_operations(["setup", "c1"]),
{:add_cell_doctest_report, @cid, "c1", %{status: :running, line: 5}}
])
operation = {:erase_outputs, @cid}
empty_map = %{}
assert {:ok, %{cell_infos: %{"c1" => %{eval: %{doctest_reports: ^empty_map}}}}, []} =
Data.apply_operation(data, operation)
end
end
describe "apply_operation/2 given :set_notebook_name" do

View file

@ -13,7 +13,8 @@ defmodule Livebook.SessionTest do
interrupted: false,
evaluation_time_ms: 10,
identifiers_used: [],
identifiers_defined: %{}
identifiers_defined: %{},
code_markers: []
}
describe "file_name_for_download/1" do