From 2ff327e742229458084fcd86a68f70524279d4c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Wed, 30 Jun 2021 17:48:27 +0200 Subject: [PATCH] Implement cells bin (#414) * Implement cells bin * Temporarily keep cells source * Send complete bin entries from session to clients when a cell gets removed * Polish styles * Hydrate bin entries on section deletion as well --- assets/css/js_interop.css | 2 +- assets/js/app.js | 4 + assets/js/cell/live_editor/monaco.js | 12 ++ assets/js/cell/markdown.js | 15 +- assets/js/clip_copy/index.js | 39 +++++ assets/js/highlight/index.js | 39 +++++ assets/js/session/index.js | 19 +++ assets/js/virtualized_lines/index.js | 13 -- lib/livebook/notebook/cell.ex | 18 +- lib/livebook/session.ex | 31 ++++ lib/livebook/session/data.ex | 66 +++++++- lib/livebook/utils/time.ex | 156 ++++++++++++++++++ .../live/output/text_component.ex | 15 +- lib/livebook_web/live/session_live.ex | 51 +++++- .../live/session_live/bin_component.ex | 145 ++++++++++++++++ .../live/session_live/shortcuts_component.ex | 3 +- lib/livebook_web/router.ex | 1 + test/livebook/session/data_test.exs | 96 ++++++++++- test/livebook/session_test.exs | 13 ++ test/livebook/utils/time_test.exs | 7 + 20 files changed, 693 insertions(+), 52 deletions(-) create mode 100644 assets/js/clip_copy/index.js create mode 100644 assets/js/highlight/index.js create mode 100644 lib/livebook/utils/time.ex create mode 100644 lib/livebook_web/live/session_live/bin_component.ex create mode 100644 test/livebook/utils/time_test.exs diff --git a/assets/css/js_interop.css b/assets/css/js_interop.css index 4a80d88e5..47eb1c64d 100644 --- a/assets/css/js_interop.css +++ b/assets/css/js_interop.css @@ -122,6 +122,6 @@ solely client-side operations. @apply hidden; } -[phx-hook="VirtualizedLines"]:not(:hover) [data-clipboard] { +[phx-hook="VirtualizedLines"]:not(:hover) [phx-hook="ClipCopy"] { @apply hidden; } diff --git a/assets/js/app.js b/assets/js/app.js index bb684fde1..414650b2e 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -22,6 +22,8 @@ import UserForm from "./user_form"; import VegaLite from "./vega_lite"; import Timer from "./timer"; import MarkdownRenderer from "./markdown_renderer"; +import Highlight from "./highlight"; +import ClipCopy from "./clip_copy"; import morphdomCallbacks from "./morphdom_callbacks"; import { loadUserData } from "./lib/user"; @@ -37,6 +39,8 @@ const hooks = { VegaLite, Timer, MarkdownRenderer, + Highlight, + ClipCopy, }; const csrfToken = document diff --git a/assets/js/cell/live_editor/monaco.js b/assets/js/cell/live_editor/monaco.js index cca340ec8..59701b0d1 100644 --- a/assets/js/cell/live_editor/monaco.js +++ b/assets/js/cell/live_editor/monaco.js @@ -41,3 +41,15 @@ monaco.languages.registerCompletionItemProvider("elixir", { }); export default monaco; + +/** + * Highlights the given code using the same rules as in the editor. + * + * Returns a promise resolving to HTML that renders as the highlighted code. + */ +export function highlight(code, language) { + return monaco.editor.colorize(code, language).then((result) => { + // `colorize` always adds additional newline, so we remove it + return result.replace(/$/, ""); + }); +} diff --git a/assets/js/cell/markdown.js b/assets/js/cell/markdown.js index 8e0941f57..40351795f 100644 --- a/assets/js/cell/markdown.js +++ b/assets/js/cell/markdown.js @@ -2,21 +2,14 @@ import marked from "marked"; import morphdom from "morphdom"; import DOMPurify from "dompurify"; import katex from "katex"; -import monaco from "./live_editor/monaco"; +import { highlight } from "./live_editor/monaco"; // Reuse Monaco highlighter for Markdown code blocks marked.setOptions({ highlight: (code, lang, callback) => { - monaco.editor - .colorize(code, lang) - .then((result) => { - // `colorize` always adds additional newline, so we remove it - result = result.replace(/$/, ""); - callback(null, result); - }) - .catch((error) => { - callback(error, null); - }); + highlight(code, lang) + .then((html) => callback(null, html)) + .catch((error) => callback(error, null)); }, }); diff --git a/assets/js/clip_copy/index.js b/assets/js/clip_copy/index.js new file mode 100644 index 000000000..44c9d1f88 --- /dev/null +++ b/assets/js/clip_copy/index.js @@ -0,0 +1,39 @@ +import { getAttributeOrThrow } from "../lib/attribute"; + +/** + * A hook adding click handler that copies text from the target + * element to clipboard. + * + * Configuration: + * + * * `data-target-id` - HTML id of the element whose `innerText` is copied + */ +const ClipCopy = { + mounted() { + this.props = getProps(this); + + this.el.addEventListener("click", (event) => { + const target = document.getElementById(this.props.targetId); + + if (target) { + const text = target.innerText; + + if ("clipboard" in navigator) { + navigator.clipboard.writeText(text); + } + } + }); + }, + + updated() { + this.props = getProps(this); + }, +}; + +function getProps(hook) { + return { + targetId: getAttributeOrThrow(hook.el, "data-target-id"), + }; +} + +export default ClipCopy; diff --git a/assets/js/highlight/index.js b/assets/js/highlight/index.js new file mode 100644 index 000000000..00b070a42 --- /dev/null +++ b/assets/js/highlight/index.js @@ -0,0 +1,39 @@ +import { getAttributeOrThrow } from "../lib/attribute"; +import { highlight } from "../cell/live_editor/monaco"; + +/** + * A hook used to highlight source code in the root element. + * + * Configuration: + * + * * `data-language` - language of the source code + */ +const Highlight = { + mounted() { + this.props = getProps(this); + + highlightIn(this.el, this.props.language); + }, + + updated() { + this.props = getProps(this); + + highlightIn(this.el, this.props.language); + }, +}; + +function getProps(hook) { + return { + language: getAttributeOrThrow(hook.el, "data-language"), + }; +} + +function highlightIn(element, language) { + const code = element.innerText; + + highlight(code, language).then((html) => { + element.innerHTML = html; + }); +} + +export default Highlight; diff --git a/assets/js/session/index.js b/assets/js/session/index.js index aa2107703..ad9cd8c3e 100644 --- a/assets/js/session/index.js +++ b/assets/js/session/index.js @@ -140,6 +140,10 @@ const Session = { } ); + this.handleEvent("cell_restored", ({ cell_id: cellId }) => { + handleCellRestored(this, cellId); + }); + this.handleEvent("cell_moved", ({ cell_id }) => { handleCellMoved(this, cell_id); }); @@ -293,6 +297,8 @@ function handleDocumentKeyDown(hook, event) { toggleClientsList(hook); } else if (keyBuffer.tryMatch(["s", "r"])) { showNotebookRuntimeSettings(hook); + } else if (keyBuffer.tryMatch(["s", "b"])) { + showBin(hook); } else if (keyBuffer.tryMatch(["e", "x"])) { cancelFocusedCellEvaluation(hook); } else if (keyBuffer.tryMatch(["?"])) { @@ -327,6 +333,11 @@ function handleDocumentKeyDown(hook, event) { * (e.g. if the user starts selecting some text within the editor) */ function handleDocumentMouseDown(hook, event) { + // If the click is outside the notebook element, keep the focus as is + if (!event.target.closest(`[data-element="notebook"]`)) { + return; + } + // If click targets a clickable element that awaits mouse up, keep the focus as is if (event.target.closest(`a, button`)) { // If the pencil icon is clicked, enter insert mode @@ -533,6 +544,10 @@ function showNotebookRuntimeSettings(hook) { hook.pushEvent("show_runtime_settings", {}); } +function showBin(hook) { + hook.pushEvent("show_bin", {}); +} + function saveNotebook(hook) { hook.pushEvent("save", {}); } @@ -706,6 +721,10 @@ function handleCellDeleted(hook, cellId, siblingCellId) { } } +function handleCellRestored(hook, cellId) { + setFocusedCell(hook, cellId); +} + function handleCellMoved(hook, cellId) { if (hook.state.focusedCellId === cellId) { globalPubSub.broadcast("cells", { type: "cell_moved", cellId }); diff --git a/assets/js/virtualized_lines/index.js b/assets/js/virtualized_lines/index.js index 48de1ba1c..aef240577 100644 --- a/assets/js/virtualized_lines/index.js +++ b/assets/js/virtualized_lines/index.js @@ -24,7 +24,6 @@ import { getLineHeight } from "../lib/utils"; * * one annotated with `data-content` where the visible elements are rendered, * it should contain any styling relevant for the container * - * Also a `data-clipboard` child button is used for triggering copy-to-clipboard. */ const VirtualizedLines = { mounted() { @@ -64,18 +63,6 @@ const VirtualizedLines = { this.state.contentElement, config ); - - this.el - .querySelector("[data-clipboard]") - .addEventListener("click", (event) => { - const content = Array.from(this.state.templateElement.children) - .map((child) => child.innerText) - .join(""); - - if ("clipboard" in navigator) { - navigator.clipboard.writeText(content); - } - }); }, updated() { diff --git a/lib/livebook/notebook/cell.ex b/lib/livebook/notebook/cell.ex index 1d52c2dcb..90c5e5289 100644 --- a/lib/livebook/notebook/cell.ex +++ b/lib/livebook/notebook/cell.ex @@ -17,20 +17,32 @@ defmodule Livebook.Notebook.Cell do ## Recognised entries - * `disable_formatting` - whether this particular cell should no be automatically formatted. - Relevant for Elixir cells only. + * `disable_formatting` - whether this particular cell should + not be automatically formatted. Relevant for Elixir cells only. """ @type metadata :: %{String.t() => term()} @type t :: Cell.Elixir.t() | Cell.Markdown.t() | Cell.Input.t() + @type type :: :markdown | :elixir | :input + @doc """ Returns an empty cell of the given type. """ - @spec new(:markdown | :elixir | :input) :: t() + @spec new(type()) :: t() def new(type) def new(:markdown), do: Cell.Markdown.new() def new(:elixir), do: Cell.Elixir.new() def new(:input), do: Cell.Input.new() + + @doc """ + Returns an atom representing the type of the given cell. + """ + @spec type(t()) :: type() + def type(cell) + + def type(%Cell.Elixir{}), do: :elixir + def type(%Cell.Markdown{}), do: :markdown + def type(%Cell.Input{}), do: :input end diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 6d897b6df..4d8baf9be 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -148,6 +148,14 @@ defmodule Livebook.Session do GenServer.cast(name(session_id), {:delete_cell, self(), cell_id}) end + @doc """ + Asynchronously sends cell restoration request to the server. + """ + @spec restore_cell(id(), Cell.id()) :: :ok + def restore_cell(session_id, cell_id) do + GenServer.cast(name(session_id), {:restore_cell, self(), cell_id}) + end + @doc """ Asynchronously sends cell move request to the server. """ @@ -382,6 +390,11 @@ defmodule Livebook.Session do {:noreply, handle_operation(state, operation)} end + def handle_cast({:restore_cell, client_pid, cell_id}, state) do + operation = {:restore_cell, client_pid, cell_id} + {:noreply, handle_operation(state, operation)} + end + def handle_cast({:move_cell, client_pid, cell_id, offset}, state) do operation = {:move_cell, client_pid, cell_id, offset} {:noreply, handle_operation(state, operation)} @@ -679,6 +692,24 @@ defmodule Livebook.Session do state end + defp after_operation(state, _prev_state, {:delete_cell, _client_pid, cell_id}) do + entry = Enum.find(state.data.bin_entries, fn entry -> entry.cell.id == cell_id end) + # The session LV drops cell's source, so we send them + # the complete bin entry to override + broadcast_message(state.session_id, {:hydrate_bin_entries, [entry]}) + + state + end + + defp after_operation(state, prev_state, {:delete_section, _client_pid, section_id, true}) do + {:ok, section} = Notebook.fetch_section(prev_state.data.notebook, section_id) + cell_ids = Enum.map(section.cells, & &1.id) + entries = Enum.filter(state.data.bin_entries, fn entry -> entry.cell.id in cell_ids end) + broadcast_message(state.session_id, {:hydrate_bin_entries, entries}) + + state + end + defp after_operation(state, _prev_state, _operation), do: state defp handle_actions(state, actions) do diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 78463b914..505126dc7 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -20,8 +20,7 @@ defmodule Livebook.Session.Data do :dirty, :section_infos, :cell_infos, - :deleted_sections, - :deleted_cells, + :bin_entries, :runtime, :clients_map, :users_map @@ -37,8 +36,7 @@ defmodule Livebook.Session.Data do dirty: boolean(), section_infos: %{Section.id() => section_info()}, cell_infos: %{Cell.id() => cell_info()}, - deleted_sections: list(Section.t()), - deleted_cells: list(Cell.t()), + bin_entries: list(cell_bin_entry()), runtime: Runtime.t() | nil, clients_map: %{pid() => User.id()}, users_map: %{User.id() => User.t()} @@ -61,6 +59,14 @@ defmodule Livebook.Session.Data do bound_to_input_ids: MapSet.t(Cell.id()) } + @type cell_bin_entry :: %{ + cell: Cell.t(), + section_id: Section.id(), + section_name: String.t(), + index: non_neg_integer(), + deleted_at: DateTime.t() + } + @type cell_revision :: non_neg_integer() @type cell_validity_status :: :fresh | :evaluated | :stale | :aborted @@ -84,6 +90,7 @@ defmodule Livebook.Session.Data do | {:insert_cell, pid(), Section.id(), index(), Cell.type(), Cell.id()} | {:delete_section, pid(), Section.id(), delete_cells :: boolean()} | {:delete_cell, pid(), Cell.id()} + | {:restore_cell, pid(), Cell.id()} | {:move_cell, pid(), Cell.id(), offset :: integer()} | {:move_section, pid(), Section.id(), offset :: integer()} | {:queue_cell_evaluation, pid(), Cell.id()} @@ -123,8 +130,7 @@ defmodule Livebook.Session.Data do dirty: false, section_infos: initial_section_infos(notebook), cell_infos: initial_cell_infos(notebook), - deleted_sections: [], - deleted_cells: [], + bin_entries: [], runtime: nil, clients_map: %{}, users_map: %{} @@ -227,6 +233,19 @@ defmodule Livebook.Session.Data do end end + def apply_operation(data, {:restore_cell, _client_pid, id}) do + with {:ok, cell_bin_entry} <- fetch_cell_bin_entry(data, id), + true <- data.notebook.sections != [] do + data + |> with_actions() + |> restore_cell(cell_bin_entry) + |> set_dirty() + |> wrap_ok() + else + _ -> :error + end + end + def apply_operation(data, {:move_cell, _client_pid, id, offset}) do with {:ok, cell, _section} <- Notebook.fetch_cell_and_section(data.notebook, id), true <- offset != 0 do @@ -506,8 +525,7 @@ defmodule Livebook.Session.Data do data_actions |> set!( notebook: Notebook.delete_section(data.notebook, section.id), - section_infos: Map.delete(data.section_infos, section.id), - deleted_sections: [%{section | cells: []} | data.deleted_sections] + section_infos: Map.delete(data.section_infos, section.id) ) end @@ -518,7 +536,16 @@ defmodule Livebook.Session.Data do |> add_action({:forget_evaluation, cell, section}) |> set!( notebook: Notebook.delete_cell(data.notebook, cell.id), - deleted_cells: [cell | data.deleted_cells] + bin_entries: [ + %{ + cell: cell, + section_id: section.id, + section_name: section.name, + index: Enum.find_index(section.cells, &(&1 == cell)), + deleted_at: DateTime.utc_now() + } + | data.bin_entries + ] ) |> delete_cell_info(cell) end @@ -528,6 +555,21 @@ defmodule Livebook.Session.Data do |> set!(cell_infos: Map.delete(data.cell_infos, cell.id)) end + defp restore_cell({data, _} = data_actions, cell_bin_entry) do + {section, index} = + case Notebook.fetch_section(data.notebook, cell_bin_entry.section_id) do + # Insert at the index of deletion, it may be no longer accurate, + # but even then makes for a good approximation and the cell can be easily moved + {:ok, section} -> {section, cell_bin_entry.index} + # Insert at the end of the notebook if the section no longer exists + :error -> {List.last(data.notebook.sections), -1} + end + + data_actions + |> insert_cell(section.id, index, cell_bin_entry.cell) + |> set!(bin_entries: List.delete(data.bin_entries, cell_bin_entry)) + end + defp move_cell({data, _} = data_actions, cell, offset) do updated_notebook = Notebook.move_cell(data.notebook, cell.id, offset) @@ -949,6 +991,12 @@ defmodule Livebook.Session.Data do %{cell_info | deltas: deltas} end + defp fetch_cell_bin_entry(data, cell_id) do + Enum.find_value(data.bin_entries, :error, fn entry -> + entry.cell.id == cell_id && {:ok, entry} + end) + end + defp add_action({data, actions}, action) do {data, actions ++ [action]} end diff --git a/lib/livebook/utils/time.ex b/lib/livebook/utils/time.ex new file mode 100644 index 000000000..75c00628e --- /dev/null +++ b/lib/livebook/utils/time.ex @@ -0,0 +1,156 @@ +defmodule Livebook.Utils.Time do + @moduledoc false + + # A simplified version of https://gist.github.com/tlemens/88e9b08f62150ba6082f478a4a03ac52 + + @doc """ + Formats the given point in time relatively to present. + """ + @spec time_ago_in_words(NaiveDateTime.t()) :: String.t() + def time_ago_in_words(naive_date_time) when is_struct(naive_date_time, NaiveDateTime) do + now = NaiveDateTime.utc_now() + + if NaiveDateTime.compare(naive_date_time, now) == :gt do + raise ArgumentError, "expected a datetime in the past, got: #{inspect(naive_date_time)}" + end + + distance_of_time_in_words(naive_date_time, now) + end + + @doc """ + Formats time distance between `from_ndt` and `to_ndt` + as a human-readable string. + + ## Examples + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:04]) + "less than 5 seconds" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:09]) + "less than 10 seconds" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:19]) + "less than 20 seconds" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:20]) + "half a minute" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:39]) + "half a minute" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:40]) + "less than a minute" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:15:59]) + "less than a minute" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:16:00]) + "1 minute" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:16:29]) + "1 minute" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:16:30]) + "2 minutes" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:58:30]) + "44 minutes" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 18:59:30]) + "about 1 hour" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-20 19:59:30]) + "about 2 hours" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-21 18:14:00]) + "about 24 hours" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-21 18:15:00]) + "1 day" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-06-22 18:15:00]) + "2 days" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2020-07-22 18:15:00]) + "about 1 month" + + iex> Livebook.Utils.Time.distance_of_time_in_words(~N[2020-06-20 18:15:00], ~N[2021-08-22 18:15:00]) + "about 14 months" + """ + @spec distance_of_time_in_words(NaiveDateTime.t(), NaiveDateTime.t()) :: String.t() + def distance_of_time_in_words(from_ndt, to_ndt) + when is_struct(from_ndt, NaiveDateTime) and is_struct(to_ndt, NaiveDateTime) do + duration_seconds = NaiveDateTime.diff(to_ndt, from_ndt) + + {:seconds, duration_seconds} + |> maybe_convert_to_minutes() + |> duration_in_words() + end + + defp maybe_convert_to_minutes({:seconds, seconds}) when seconds > 59 do + {:minutes, round(seconds / 60)} + end + + defp maybe_convert_to_minutes(duration), do: duration + + defp duration_in_words({:seconds, seconds}) when seconds in 0..4 do + "less than 5 seconds" + end + + defp duration_in_words({:seconds, seconds}) when seconds in 5..9 do + "less than 10 seconds" + end + + defp duration_in_words({:seconds, seconds}) when seconds in 10..19 do + "less than 20 seconds" + end + + defp duration_in_words({:seconds, seconds}) when seconds in 20..39 do + "half a minute" + end + + defp duration_in_words({:seconds, seconds}) when seconds in 40..59 do + "less than a minute" + end + + defp duration_in_words({:minutes, minutes}) when minutes == 1 do + "1 minute" + end + + defp duration_in_words({:minutes, minutes}) when minutes in 2..44 do + "#{minutes} minutes" + end + + defp duration_in_words({:minutes, minutes}) when minutes in 45..89 do + "about 1 hour" + end + + # 90 mins up to 24 hours + defp duration_in_words({:minutes, minutes}) when minutes in 90..1439 do + "about #{round(minutes / 60)} hours" + end + + # 24 hours up to 42 hours + defp duration_in_words({:minutes, minutes}) when minutes in 1440..2519 do + "1 day" + end + + # 42 hours up to 30 days + defp duration_in_words({:minutes, minutes}) when minutes in 2520..43_199 do + "#{round(minutes / 1440)} days" + end + + # 30 days up to 45 days + defp duration_in_words({:minutes, minutes}) when minutes in 43_200..64_799 do + "about 1 month" + end + + # 45 days up to 60 days + defp duration_in_words({:minutes, minutes}) when minutes in 64_800..86_399 do + "about 2 months" + end + + defp duration_in_words({:minutes, minutes}) do + "about #{round(minutes / 43_200)} months" + end +end diff --git a/lib/livebook_web/live/output/text_component.ex b/lib/livebook_web/live/output/text_component.ex index 99f6b487c..62cddfcec 100644 --- a/lib/livebook_web/live/output/text_component.ex +++ b/lib/livebook_web/live/output/text_component.ex @@ -9,17 +9,18 @@ defmodule LivebookWeb.Output.TextComponent do phx-hook="VirtualizedLines" data-max-height="300" data-follow="<%= @follow %>"> - + <%# Add a newline to each element, so that multiple lines can be copied properly %> +
-
diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index a9b366edd..ca736ca76 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -97,6 +97,13 @@ defmodule LivebookWeb.SessionLive do label: "Runtime settings (sr)", active: @live_action == :runtime_settings }, + %{ + type: :link, + icon: "delete-bin-6-fill", + path: Routes.session_path(@socket, :bin, @session_id), + label: "Bin (sb)", + active: @live_action == :bin + }, %{type: :break}, %{ type: :link, @@ -297,6 +304,15 @@ defmodule LivebookWeb.SessionLive do is_first: @section.id == @first_section_id, return_to: Routes.session_path(@socket, :page, @session_id) %> <% end %> + + <%= if @live_action == :bin do %> + <%= live_modal LivebookWeb.SessionLive.BinComponent, + id: :bin_modal, + modal_class: "w-full max-w-4xl", + session_id: @session_id, + bin_entries: @data_view.bin_entries, + return_to: Routes.session_path(@socket, :page, @session_id) %> + <% end %> """ end @@ -545,6 +561,11 @@ defmodule LivebookWeb.SessionLive do )} end + def handle_event("show_bin", %{}, socket) do + {:noreply, + push_patch(socket, to: Routes.session_path(socket, :bin, socket.assigns.session_id))} + end + def handle_event("completion_request", %{"hint" => hint, "cell_id" => cell_id}, socket) do data = socket.private.data @@ -631,6 +652,25 @@ defmodule LivebookWeb.SessionLive do {:noreply, put_flash(socket, :info, message)} end + def handle_info({:hydrate_bin_entries, hydrated_entries}, socket) do + hydrated_entries_map = Map.new(hydrated_entries, fn entry -> {entry.cell.id, entry} end) + + data = + Map.update!(socket.private.data, :bin_entries, fn bin_entries -> + Enum.map(bin_entries, fn entry -> + case Map.fetch(hydrated_entries_map, entry.cell.id) do + {:ok, hydrated_entry} -> hydrated_entry + :error -> entry + end + end) + end) + + {:noreply, + socket + |> assign_private(data: data) + |> assign(data_view: data_to_view(data))} + end + def handle_info(:session_closed, socket) do {:noreply, socket @@ -732,6 +772,14 @@ defmodule LivebookWeb.SessionLive do push_event(socket, "cell_deleted", %{cell_id: cell_id, sibling_cell_id: sibling_cell_id}) end + defp after_operation(socket, _prev_socket, {:restore_cell, client_pid, cell_id}) do + if client_pid == self() do + push_event(socket, "cell_restored", %{cell_id: cell_id}) + else + socket + end + end + defp after_operation(socket, _prev_socket, {:move_cell, client_pid, cell_id, _offset}) do if client_pid == self() do push_event(socket, "cell_moved", %{cell_id: cell_id}) @@ -831,7 +879,8 @@ defmodule LivebookWeb.SessionLive do data.clients_map |> Enum.map(fn {client_pid, user_id} -> {client_pid, data.users_map[user_id]} end) |> Enum.sort_by(fn {_client_pid, user} -> user.name end), - section_views: section_views(data.notebook.sections, data) + section_views: section_views(data.notebook.sections, data), + bin_entries: data.bin_entries } end diff --git a/lib/livebook_web/live/session_live/bin_component.ex b/lib/livebook_web/live/session_live/bin_component.ex new file mode 100644 index 000000000..0b8bf698a --- /dev/null +++ b/lib/livebook_web/live/session_live/bin_component.ex @@ -0,0 +1,145 @@ +defmodule LivebookWeb.SessionLive.BinComponent do + use LivebookWeb, :live_component + + alias Livebook.Notebook.Cell + + @initial_limit 10 + @limit_step 10 + + @impl true + def mount(socket) do + {:ok, assign(socket, search: "", limit: @initial_limit)} + end + + @impl true + def update(assigns, socket) do + {bin_entries, assigns} = Map.pop(assigns, :bin_entries) + + # Only show text cells, as they have an actual content + bin_entries = + Enum.filter(bin_entries, fn entry -> + Cell.type(entry.cell) in [:markdown, :elixir] + end) + + {:ok, + socket + |> assign(:bin_entries, bin_entries) + |> assign(assigns) + |> assign_matching_entries()} + end + + @impl true + def render(assigns) do + ~L""" +
+

+ Bin +

+
+

+ Here you can find all the cells deleted within this session. +

+ <%= if @bin_entries == [] do %> +
+
+ <%= remix_icon("windy-line", class: "text-gray-400 text-xl") %> +
+
+ There are currently no cells in the bin. +
+
+ <% else %> +
+ +
+
+ <%= for %{cell: cell} = entry <- Enum.take(@matching_entries, @limit) do %> +
+
+

+ <%= Cell.type(cell) |> Atom.to_string() |> String.capitalize() %> cell + deleted from “<%= entry.section_name %>” section + + <%= entry.deleted_at |> DateTime.to_naive() |> Livebook.Utils.Time.time_ago_in_words() %> ago + +

+
+ + + + + + +
+
+
+
<%= cell.source %>
+
+
+ <% end %> + <%= if length(@matching_entries) > @limit do %> +
+ +
+ <% end %> +
+ <% end %> +
+
+ """ + end + + @impl true + def handle_event("search", %{"search" => search}, socket) do + {:noreply, assign(socket, search: search, limit: @initial_limit) |> assign_matching_entries()} + end + + def handle_event("more", %{}, socket) do + {:noreply, assign(socket, limit: socket.assigns.limit + @limit_step)} + end + + def handle_event("restore", %{"cell_id" => cell_id}, socket) do + Livebook.Session.restore_cell(socket.assigns.session_id, cell_id) + {:noreply, push_patch(socket, to: socket.assigns.return_to)} + end + + defp assign_matching_entries(socket) do + matching_entries = filter_matching(socket.assigns.bin_entries, socket.assigns.search) + assign(socket, matching_entries: matching_entries) + end + + defp filter_matching(entries, search) do + parts = + search + |> String.split() + |> Enum.map(fn part -> ~r/#{Regex.escape(part)}/i end) + + Enum.filter(entries, fn entry -> + Enum.all?(parts, fn part -> + entry.cell.source =~ part + end) + end) + end +end diff --git a/lib/livebook_web/live/session_live/shortcuts_component.ex b/lib/livebook_web/live/session_live/shortcuts_component.ex index bfbb61f8e..7a7670a38 100644 --- a/lib/livebook_web/live/session_live/shortcuts_component.ex +++ b/lib/livebook_web/live/session_live/shortcuts_component.ex @@ -32,7 +32,8 @@ defmodule LivebookWeb.SessionLive.ShortcutsComponent do %{seq: ["e", "x"], desc: "Cancel cell evaluation"}, %{seq: ["s", "s"], desc: "Toggle sections panel"}, %{seq: ["s", "u"], desc: "Toggle users panel"}, - %{seq: ["s", "r"], desc: "Show runtime settings"} + %{seq: ["s", "r"], desc: "Show runtime settings"}, + %{seq: ["s", "b"], desc: "Show bin"} ], universal: [ %{seq: ["ctrl", "s"], press_all: true, desc: "Save notebook"} diff --git a/lib/livebook_web/router.ex b/lib/livebook_web/router.ex index 9ac71c529..291412451 100644 --- a/lib/livebook_web/router.ex +++ b/lib/livebook_web/router.ex @@ -31,6 +31,7 @@ defmodule LivebookWeb.Router do live "/sessions/:id/shortcuts", SessionLive, :shortcuts live "/sessions/:id/settings/runtime", SessionLive, :runtime_settings live "/sessions/:id/settings/file", SessionLive, :file_settings + live "/sessions/:id/bin", SessionLive, :bin live "/sessions/:id/cell-settings/:cell_id", SessionLive, :cell_settings live "/sessions/:id/cell-upload/:cell_id", SessionLive, :cell_upload live "/sessions/:id/delete-section/:section_id", SessionLive, :delete_section diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index 40e76f3e5..899012d99 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -142,7 +142,7 @@ defmodule Livebook.Session.DataTest do assert :error = Data.apply_operation(data, operation) end - test "removes the section from notebook and section info, adds to deleted sections" do + test "removes the section from notebook and section info" do data = data_after_operations!([ {:insert_section, self(), 0, "s1"} @@ -156,8 +156,7 @@ defmodule Livebook.Session.DataTest do notebook: %{ sections: [] }, - section_infos: ^empty_map, - deleted_sections: [%{id: "s1"}] + section_infos: ^empty_map }, []} = Data.apply_operation(data, operation) end @@ -188,7 +187,7 @@ defmodule Livebook.Session.DataTest do notebook: %{ sections: [%{id: "s1", cells: [%{id: "c1"}, %{id: "c2"}]}] }, - deleted_cells: [] + bin_entries: [] }, []} = Data.apply_operation(data, operation) end @@ -208,7 +207,15 @@ defmodule Livebook.Session.DataTest do notebook: %{ sections: [%{id: "s1", cells: [%{id: "c1"}]}] }, - deleted_cells: [%{id: "c2"}] + bin_entries: [ + %{ + cell: %{id: "c2"}, + section_id: "s2", + section_name: "Section", + index: 0, + deleted_at: _ + } + ] }, [{:forget_evaluation, %{id: "c2"}, %{id: "s2"}}]} = Data.apply_operation(data, operation) @@ -281,7 +288,15 @@ defmodule Livebook.Session.DataTest do sections: [%{cells: []}] }, cell_infos: ^empty_map, - deleted_cells: [%{id: "c1"}] + bin_entries: [ + %{ + cell: %{id: "c1"}, + section_id: "s1", + section_name: "Section", + index: 0, + deleted_at: _ + } + ] }, _actions} = Data.apply_operation(data, operation) end @@ -360,6 +375,75 @@ defmodule Livebook.Session.DataTest do end end + describe "apply_operation/2 given :restore_cell" do + test "returns an error if cell with the given id is not in the bin" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"} + ]) + + operation = {:restore_cell, self(), "c1"} + assert :error = Data.apply_operation(data, operation) + end + + test "returns an error if there are no sections to restore to" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:delete_section, self(), "s1", true} + ]) + + operation = {:restore_cell, self(), "c1"} + assert :error = Data.apply_operation(data, operation) + end + + test "if the original section exists, restores cell at the index of deletion" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_cell, self(), "s1", 1, :elixir, "c2"}, + {:insert_cell, self(), "s1", 2, :elixir, "c3"}, + {:delete_cell, self(), "c2"} + ]) + + operation = {:restore_cell, self(), "c2"} + + assert {:ok, + %{ + notebook: %{ + sections: [%{id: "s1", cells: [%{id: "c1"}, %{id: "c2"}, %{id: "c3"}]}] + }, + cell_infos: %{"c2" => %{}}, + bin_entries: [] + }, _actions} = Data.apply_operation(data, operation) + end + + test "if the original section does not exist, restores cell at the end of the notebook" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_section, self(), 1, "s2"}, + {:insert_cell, self(), "s2", 0, :elixir, "c2"}, + {:delete_section, self(), "s1", true} + ]) + + operation = {:restore_cell, self(), "c1"} + + assert {:ok, + %{ + notebook: %{ + sections: [%{id: "s2", cells: [%{id: "c2"}, %{id: "c1"}]}] + }, + cell_infos: %{"c2" => %{}}, + bin_entries: [] + }, _actions} = Data.apply_operation(data, operation) + end + end + describe "apply_operation/2 given :move_cell" do test "returns an error given invalid cell id" do data = Data.new() diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs index 747383abc..6d06d564e 100644 --- a/test/livebook/session_test.exs +++ b/test/livebook/session_test.exs @@ -61,6 +61,19 @@ defmodule Livebook.SessionTest do end end + describe "restore_cell/2" do + test "sends a restore opreation to subscribers", %{session_id: session_id} do + Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") + pid = self() + + {_section_id, cell_id} = insert_section_and_cell(session_id) + Session.delete_cell(session_id, cell_id) + + Session.restore_cell(session_id, cell_id) + assert_receive {:operation, {:restore_cell, ^pid, ^cell_id}} + end + end + describe "queue_cell_evaluation/2" do test "sends a queue evaluation operation to subscribers", %{session_id: session_id} do Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") diff --git a/test/livebook/utils/time_test.exs b/test/livebook/utils/time_test.exs new file mode 100644 index 000000000..3695fecd9 --- /dev/null +++ b/test/livebook/utils/time_test.exs @@ -0,0 +1,7 @@ +defmodule Livebook.Utils.TimeTest do + use ExUnit.Case, async: true + + alias Livebook.Utils + + doctest Utils.Time +end