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 smart cell indicator when source changes on start #1851

Merged
merged 3 commits into from
Apr 8, 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
61 changes: 36 additions & 25 deletions lib/livebook/delta.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ defmodule Livebook.Delta do
alias Livebook.Delta
alias Livebook.Delta.{Operation, Transformation}

@typedoc """
Delta carries a list of consecutive operations.

Note that we keep the operations in reversed order for efficiency.
"""
@type t :: %Delta{ops: list(Operation.t())}

@doc """
Expand Down Expand Up @@ -73,12 +78,7 @@ defmodule Livebook.Delta do
"""
@spec append(t(), Operation.t()) :: t()
def append(delta, op) do
Map.update!(delta, :ops, fn ops ->
ops
|> Enum.reverse()
|> compact(op)
|> Enum.reverse()
end)
Map.update!(delta, :ops, &compact(&1, op))
end

defp compact(ops, {:insert, ""}), do: ops
Expand Down Expand Up @@ -110,18 +110,23 @@ defmodule Livebook.Delta do
Removes trailing retain operations from the given delta.
"""
@spec trim(t()) :: t()
def trim(%Delta{ops: []} = delta), do: delta

def trim(delta) do
case List.last(delta.ops) do
{:retain, _} ->
Map.update!(delta, :ops, fn ops ->
ops |> Enum.reverse() |> tl() |> Enum.reverse()
end)

_ ->
delta
end
def trim(%Delta{ops: [{:retain, _} | ops]} = delta), do: %{delta | ops: ops}
def trim(delta), do: delta

@doc """
Checks if the delta has no changes.
"""
@spec empty?(t()) :: boolean()
def empty?(delta) do
trim(delta).ops == []
end

@doc """
Returns data operations in the order in which they apply.
"""
@spec operations(t()) :: list(Operation.t())
def operations(delta) do
Enum.reverse(delta.ops)
end

@doc """
Expand All @@ -130,30 +135,36 @@ defmodule Livebook.Delta do

## Examples

iex> delta = %Livebook.Delta{ops: [retain: 2, insert: "hey", delete: 3]}
iex> delta = Delta.new([retain: 2, insert: "hey", delete: 3])
iex> Livebook.Delta.to_compressed(delta)
[2, "hey", -3]

"""
@spec to_compressed(t()) :: list(Operation.compressed_t())
def to_compressed(delta) do
Enum.map(delta.ops, &Operation.to_compressed/1)
delta.ops
|> Enum.reverse()
|> Enum.map(&Operation.to_compressed/1)
end

@doc """
Builds a new delta from the given compact representation.

## Examples

iex> Livebook.Delta.from_compressed([2, "hey", -3])
%Livebook.Delta{ops: [retain: 2, insert: "hey", delete: 3]}
iex> delta = Livebook.Delta.from_compressed([2, "hey", -3])
iex> Livebook.Delta.operations(delta)
[retain: 2, insert: "hey", delete: 3]

"""
@spec from_compressed(list(Operation.compressed_t())) :: t()
def from_compressed(list) do
list
|> Enum.map(&Operation.from_compressed/1)
|> new()
ops =
list
|> Enum.map(&Operation.from_compressed/1)
|> Enum.reverse()

%Delta{ops: ops}
end

defdelegate transform(left, right, priority), to: Transformation
Expand Down
2 changes: 1 addition & 1 deletion lib/livebook/delta/transformation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ defmodule Livebook.Delta.Transformation do
"""
@spec transform(Delta.t(), Delta.t(), priority()) :: Delta.t()
def transform(left, right, priority) do
do_transform(left.ops, right.ops, priority, Delta.new())
do_transform(Delta.operations(left), Delta.operations(right), priority, Delta.new())
|> Delta.trim()
end

Expand Down
3 changes: 2 additions & 1 deletion lib/livebook/js_interop.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ defmodule Livebook.JSInterop do
def apply_delta_to_string(delta, string) do
code_units = string_to_utf16_code_units(string)

delta.ops
delta
|> Delta.operations()
|> apply_to_code_units(code_units)
|> utf16_code_units_to_string()
end
Expand Down
12 changes: 12 additions & 0 deletions lib/livebook/session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1858,6 +1858,18 @@ defmodule Livebook.Session do
state
end

defp after_operation(
state,
_prev_state,
{:smart_cell_started, _client_id, cell_id, delta, _chunks, _js_view, _editor}
) do
unless Delta.empty?(delta) do
hydrate_cell_source_digest(state, cell_id, :primary)
end

state
end

defp after_operation(
state,
_prev_state,
Expand Down
16 changes: 8 additions & 8 deletions test/livebook/delta_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,39 +8,39 @@ defmodule Livebook.DeltaTest do

describe "append/2" do
test "ignores empty operations" do
assert Delta.append(Delta.new(), {:insert, ""}) == %Delta{ops: []}
assert Delta.append(Delta.new(), {:retain, 0}) == %Delta{ops: []}
assert Delta.append(Delta.new(), {:delete, 0}) == %Delta{ops: []}
assert Delta.new() |> Delta.append({:insert, ""}) |> Delta.operations() == []
assert Delta.new() |> Delta.append({:retain, 0}) |> Delta.operations() == []
assert Delta.new() |> Delta.append({:delete, 0}) |> Delta.operations() == []
end

test "given empty delta just appends the operation" do
delta = Delta.new()
op = Operation.insert("cats")
assert Delta.append(delta, op) == %Delta{ops: [insert: "cats"]}
assert delta |> Delta.append(op) |> Delta.operations() == [insert: "cats"]
end

test "merges consecutive inserts" do
delta = Delta.new() |> Delta.insert("cats")
op = Operation.insert(" rule")
assert Delta.append(delta, op) == %Delta{ops: [insert: "cats rule"]}
assert delta |> Delta.append(op) |> Delta.operations() == [insert: "cats rule"]
end

test "merges consecutive retains" do
delta = Delta.new() |> Delta.retain(2)
op = Operation.retain(2)
assert Delta.append(delta, op) == %Delta{ops: [retain: 4]}
assert delta |> Delta.append(op) |> Delta.operations() == [retain: 4]
end

test "merges consecutive delete" do
delta = Delta.new() |> Delta.delete(2)
op = Operation.delete(2)
assert Delta.append(delta, op) == %Delta{ops: [delete: 4]}
assert delta |> Delta.append(op) |> Delta.operations() == [delete: 4]
end

test "given insert appended after delete, swaps the operations" do
delta = Delta.new() |> Delta.delete(2)
op = Operation.insert("cats")
assert Delta.append(delta, op) == %Delta{ops: [insert: "cats", delete: 2]}
assert delta |> Delta.append(op) |> Delta.operations() == [insert: "cats", delete: 2]
end
end
end
7 changes: 6 additions & 1 deletion test/livebook/session_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ defmodule Livebook.SessionTest do
end

test "pings the smart cell before evaluation to await all incoming messages" do
smart_cell = %{Notebook.Cell.new(:smart) | kind: "text", source: "1"}
smart_cell = %{Notebook.Cell.new(:smart) | kind: "text", source: ""}
notebook = %{Notebook.new() | sections: [%{Notebook.Section.new() | cells: [smart_cell]}]}
session = start_session(notebook: notebook)

Expand All @@ -964,6 +964,11 @@ defmodule Livebook.SessionTest do
%{source: "1", js_view: %{pid: self(), ref: "ref"}, editor: nil}}
)

# Sends digest to clients when the source is different
cell_id = smart_cell.id
new_digest = :erlang.md5("1")
assert_receive {:hydrate_cell_source_digest, ^cell_id, :primary, ^new_digest}

Session.queue_cell_evaluation(session.pid, smart_cell.id)

send(session.pid, {:runtime_evaluation_response, "setup", {:ok, ""}, @eval_meta})
Expand Down