diff --git a/lib/livebook/live_markdown/export.ex b/lib/livebook/live_markdown/export.ex index 5c5f57138..56d28da4e 100644 --- a/lib/livebook/live_markdown/export.ex +++ b/lib/livebook/live_markdown/export.ex @@ -9,12 +9,13 @@ defmodule Livebook.LiveMarkdown.Export do ## Options * `:include_outputs` - whether to render cell outputs. - Only textual outputs are included. Defaults to `false`. + Only textual outputs are included. Defaults to the + value of `:persist_outputs` notebook attribute. """ @spec notebook_to_markdown(Notebook.t(), keyword()) :: String.t() def notebook_to_markdown(notebook, opts \\ []) do ctx = %{ - include_outputs?: Keyword.get(opts, :include_outputs, false) + include_outputs?: Keyword.get(opts, :include_outputs, notebook.persist_outputs) } iodata = render_notebook(notebook, ctx) @@ -33,8 +34,8 @@ defmodule Livebook.LiveMarkdown.Export do |> prepend_metadata(metadata) end - defp notebook_metadata(_notebook) do - %{} + defp notebook_metadata(notebook) do + put_unless_implicit(%{}, persist_outputs: notebook.persist_outputs) end defp render_section(section, notebook, ctx) do diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex index 2d6f38d52..d3462a3b1 100644 --- a/lib/livebook/live_markdown/import.ex +++ b/lib/livebook/live_markdown/import.ex @@ -282,8 +282,14 @@ defmodule Livebook.LiveMarkdown.Import do end) end - defp notebook_metadata_to_attrs(_metadata) do - %{} + defp notebook_metadata_to_attrs(metadata) do + Enum.reduce(metadata, %{}, fn + {"persist_outputs", persist_outputs}, attrs -> + Map.put(attrs, :persist_outputs, persist_outputs) + + _entry, attrs -> + attrs + end) end defp section_metadata_to_attrs(metadata) do diff --git a/lib/livebook/notebook.ex b/lib/livebook/notebook.ex index 2857a2ef4..30375752e 100644 --- a/lib/livebook/notebook.ex +++ b/lib/livebook/notebook.ex @@ -13,7 +13,7 @@ defmodule Livebook.Notebook do # A notebook is divided into a number of *sections*, each # containing a number of *cells*. - defstruct [:name, :version, :sections] + defstruct [:name, :version, :sections, :persist_outputs] alias Livebook.Notebook.{Section, Cell} alias Livebook.Utils.Graph @@ -22,7 +22,8 @@ defmodule Livebook.Notebook do @type t :: %__MODULE__{ name: String.t(), version: String.t(), - sections: list(Section.t()) + sections: list(Section.t()), + persist_outputs: boolean() } @version "1.0" @@ -35,7 +36,8 @@ defmodule Livebook.Notebook do %__MODULE__{ name: "Untitled notebook", version: @version, - sections: [] + sections: [], + persist_outputs: false } end diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 9c0566b1a..4a5b0b626 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -150,6 +150,14 @@ defmodule Livebook.Session do GenServer.call(name(session_id), :get_notebook) end + @doc """ + Asynchronously sends notebook attributes update to the server. + """ + @spec set_notebook_attributes(id(), map()) :: :ok + def set_notebook_attributes(session_id, attrs) do + GenServer.cast(name(session_id), {:set_notebook_attributes, self(), attrs}) + end + @doc """ Asynchronously sends section insertion request to the server. """ @@ -427,6 +435,11 @@ defmodule Livebook.Session do end @impl true + def handle_cast({:set_notebook_attributes, client_pid, attrs}, state) do + operation = {:set_notebook_attributes, client_pid, attrs} + {:noreply, handle_operation(state, operation)} + end + def handle_cast({:insert_section, client_pid, index}, state) do # Include new id in the operation, so it's reproducible operation = {:insert_section, client_pid, index, Utils.random_id()} diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 4c2f46aef..071e449be 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -107,7 +107,8 @@ defmodule Livebook.Session.Data do # and is passed for informative purposes only. @type operation :: - {:insert_section, pid(), index(), Section.id()} + {:set_notebook_attributes, pid(), map()} + | {:insert_section, pid(), index(), Section.id()} | {:insert_section_into, pid(), Section.id(), index(), Section.id()} | {:set_section_parent, pid(), Section.id(), parent_id :: Section.id()} | {:unset_section_parent, pid(), Section.id()} @@ -202,6 +203,18 @@ defmodule Livebook.Session.Data do @spec apply_operation(t(), operation()) :: {:ok, t(), list(action())} | :error def apply_operation(data, operation) + def apply_operation(data, {:set_notebook_attributes, _client_pid, attrs}) do + with true <- valid_attrs_for?(data.notebook, attrs) do + data + |> with_actions() + |> set_notebook_attributes(attrs) + |> set_dirty() + |> wrap_ok() + else + _ -> :error + end + end + def apply_operation(data, {:insert_section, _client_pid, index, id}) do section = %{Section.new() | id: id} @@ -375,6 +388,7 @@ defmodule Livebook.Session.Data do data |> with_actions() |> add_cell_evaluation_output(cell, output) + |> mark_dirty_if_persisting_outputs() |> wrap_ok() else _ -> :error @@ -391,6 +405,7 @@ defmodule Livebook.Session.Data do |> compute_snapshots_and_validity() |> maybe_evaluate_queued() |> compute_snapshots_and_validity() + |> mark_dirty_if_persisting_outputs() |> wrap_ok() else _ -> :error @@ -521,7 +536,7 @@ defmodule Livebook.Session.Data do def apply_operation(data, {:set_cell_attributes, _client_pid, cell_id, attrs}) do with {:ok, cell, _} <- Notebook.fetch_cell_and_section(data.notebook, cell_id), - true <- Enum.all?(attrs, fn {key, _} -> Map.has_key?(cell, key) end) do + true <- valid_attrs_for?(cell, attrs) do data |> with_actions() |> set_cell_attributes(cell, attrs) @@ -566,6 +581,11 @@ defmodule Livebook.Session.Data do defp wrap_ok({data, actions}), do: {:ok, data, actions} + defp set_notebook_attributes({data, _} = data_actions, attrs) do + data_actions + |> set!(notebook: Map.merge(data.notebook, attrs)) + end + defp insert_section({data, _} = data_actions, index, section) do data_actions |> set!( @@ -1259,6 +1279,16 @@ defmodule Livebook.Session.Data do set!(data_actions, dirty: dirty) end + defp mark_dirty_if_persisting_outputs({%{notebook: %{persist_outputs: true}}, _} = data_actions) do + set_dirty(data_actions) + end + + defp mark_dirty_if_persisting_outputs(data_actions), do: data_actions + + defp valid_attrs_for?(struct, attrs) do + Enum.all?(attrs, fn {key, _} -> Map.has_key?(struct, key) end) + end + @doc """ Find child cells bound to the given input cell. """ diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 103f7be53..d6db314b0 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -268,8 +268,8 @@ defmodule LivebookWeb.SessionLive do modal_class: "w-full max-w-4xl", return_to: Routes.session_path(@socket, :page, @session_id), session_id: @session_id, - current_path: @data_view.path, - path: @data_view.path %> + path: @data_view.path, + persist_outputs: @data_view.persist_outputs %> <% end %> <%= if @live_action == :shortcuts do %> @@ -1076,6 +1076,7 @@ defmodule LivebookWeb.SessionLive do defp data_to_view(data) do %{ path: data.path, + persist_outputs: data.notebook.persist_outputs, dirty: data.dirty, runtime: data.runtime, global_evaluation_status: global_evaluation_status(data), diff --git a/lib/livebook_web/live/session_live/export_live_markdown_component.ex b/lib/livebook_web/live/session_live/export_live_markdown_component.ex index 37328399a..d5be0824d 100644 --- a/lib/livebook_web/live/session_live/export_live_markdown_component.ex +++ b/lib/livebook_web/live/session_live/export_live_markdown_component.ex @@ -1,14 +1,14 @@ defmodule LivebookWeb.SessionLive.ExportLiveMarkdownComponent do use LivebookWeb, :live_component - @impl true - def mount(socket) do - {:ok, assign(socket, include_outputs: false)} - end - @impl true def update(assigns, socket) do - {:ok, socket |> assign(assigns) |> assign_source()} + socket = assign(socket, assigns) + + {:ok, + socket + |> assign_new(:include_outputs, fn -> socket.assigns.notebook.persist_outputs end) + |> assign_source()} end defp assign_source(%{assigns: assigns} = socket) do diff --git a/lib/livebook_web/live/session_live/persistence_component.ex b/lib/livebook_web/live/session_live/persistence_component.ex index 9ebc7985f..e0e19a573 100644 --- a/lib/livebook_web/live/session_live/persistence_component.ex +++ b/lib/livebook_web/live/session_live/persistence_component.ex @@ -10,55 +10,75 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do {:ok, assign(socket, running_paths: running_paths)} end + @impl true + def update(assigns, socket) do + {path, assigns} = Map.pop!(assigns, :path) + {persist_outputs, assigns} = Map.pop!(assigns, :persist_outputs) + + attrs = %{path: path, persist_outputs: persist_outputs} + + socket = + socket + |> assign(assigns) + |> assign(attrs: attrs, new_attrs: attrs) + + {:ok, socket} + end + @impl true def render(assigns) do ~H""" -
+

File

-
-

- Specify where the notebook should be automatically persisted. -

+
+
+
+ <.switch_checkbox + name="persist_outputs" + label="Persist outputs" + checked={@new_attrs.persist_outputs} /> +
+
<.choice_button - active={@path != nil} + active={@new_attrs.path != nil} phx-click="set_persistence_type" phx-value-type="file" phx-target={@myself}> Save to file <.choice_button - active={@path == nil} + active={@new_attrs.path == nil} phx-click="set_persistence_type" phx-value-type="memory" phx-target={@myself}> Memory only
- <%= if @path != nil do %> + <%= if @new_attrs.path != nil do %>
<%= live_component LivebookWeb.PathSelectComponent, id: "path_select", - path: @path, + path: @new_attrs.path, extnames: [LiveMarkdown.extension()], running_paths: @running_paths, phx_target: @myself, - phx_submit: if(disabled?(@path, @current_path, @running_paths), do: nil, else: "save") %> + phx_submit: if(disabled?(@new_attrs, @attrs, @running_paths), do: nil, else: "save") %>
<% end %> -
- <%= if @path != nil do %> +
+ <%= if @new_attrs.path != nil do %>
- File: <%= normalize_path(@path) %> + File: <%= normalize_path(@new_attrs.path) %>
<% end %>
-
@@ -72,27 +92,40 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do def handle_event("set_persistence_type", %{"type" => type}, socket) do path = case type do - "file" -> socket.assigns.current_path || default_path() + "file" -> socket.assigns.attrs.path || default_path() "memory" -> nil end - {:noreply, assign(socket, path: path)} + {:noreply, put_new_attr(socket, :path, path)} end def handle_event("set_path", %{"path" => path}, socket) do - {:noreply, assign(socket, path: path)} + {:noreply, put_new_attr(socket, :path, path)} end - def handle_event("save", %{}, socket) do - path = normalize_path(socket.assigns.path) - Session.set_path(socket.assigns.session_id, path) - Session.save_sync(socket.assigns.session_id) + def handle_event("set_options", %{"persist_outputs" => persist_outputs}, socket) do + persist_outputs = persist_outputs == "true" + {:noreply, put_new_attr(socket, :persist_outputs, persist_outputs)} + end + + def handle_event("save", %{}, %{assigns: assigns} = socket) do + path = normalize_path(assigns.new_attrs.path) + + if path != assigns.attrs.path do + Session.set_path(assigns.session_id, path) + end + + Session.set_notebook_attributes(assigns.session_id, %{ + persist_outputs: assigns.new_attrs.persist_outputs + }) + + Session.save_sync(assigns.session_id) running_paths = if path do - [path | socket.assigns.running_paths] + [path | assigns.running_paths] else - List.delete(socket.assigns.running_paths, path) + List.delete(assigns.running_paths, path) end # After saving the file reload the directory contents, @@ -106,6 +139,12 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do {:noreply, assign(socket, running_paths: running_paths)} end + defp put_new_attr(socket, key, value) do + new_attrs = socket.assigns.new_attrs + new_attrs = put_in(new_attrs[key], value) + assign(socket, :new_attrs, new_attrs) + end + defp default_path() do Livebook.Config.root_path() |> Path.join("notebook") end @@ -130,7 +169,11 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do end end - defp disabled?(path, current_path, running_paths) do - not path_savable?(normalize_path(path), running_paths) or normalize_path(path) == current_path + defp disabled?(new_attrs, attrs, running_paths) do + if normalize_path(new_attrs.path) == attrs.path do + new_attrs.persist_outputs == attrs.persist_outputs + else + not path_savable?(normalize_path(new_attrs.path), running_paths) + end end end diff --git a/notebook.livemd b/notebook.livemd new file mode 100644 index 000000000..c32196c3a --- /dev/null +++ b/notebook.livemd @@ -0,0 +1,3 @@ + + +# Untitled notebook diff --git a/test/livebook/live_markdown/export_test.exs b/test/livebook/live_markdown/export_test.exs index c3e8ce27b..6c6f1ec7d 100644 --- a/test/livebook/live_markdown/export_test.exs +++ b/test/livebook/live_markdown/export_test.exs @@ -668,4 +668,86 @@ defmodule Livebook.LiveMarkdown.ExportTest do assert expected_document == document end end + + test "includes outputs when notebook has :persist_outputs set" do + notebook = %{ + Notebook.new() + | name: "My Notebook", + persist_outputs: true, + sections: [ + %{ + Notebook.Section.new() + | name: "Section 1", + cells: [ + %{ + Notebook.Cell.new(:elixir) + | source: """ + IO.puts("hey")\ + """, + outputs: ["hey"] + } + ] + } + ] + } + + expected_document = """ + + + # My Notebook + + ## Section 1 + + ```elixir + IO.puts("hey") + ``` + + ```output + hey + ``` + """ + + document = Export.notebook_to_markdown(notebook) + + assert expected_document == document + end + + test "the :include_outputs option takes precedence over notebook's :persist_outputs" do + notebook = %{ + Notebook.new() + | name: "My Notebook", + persist_outputs: true, + sections: [ + %{ + Notebook.Section.new() + | name: "Section 1", + cells: [ + %{ + Notebook.Cell.new(:elixir) + | source: """ + IO.puts("hey")\ + """, + outputs: ["hey"] + } + ] + } + ] + } + + expected_document = """ + + + # My Notebook + + ## Section 1 + + ```elixir + IO.puts("hey") + ``` + """ + + document = Export.notebook_to_markdown(notebook, include_outputs: false) + + assert expected_document == document + end end diff --git a/test/livebook/live_markdown/import_test.exs b/test/livebook/live_markdown/import_test.exs index 350d6d2eb..bf56ef4c9 100644 --- a/test/livebook/live_markdown/import_test.exs +++ b/test/livebook/live_markdown/import_test.exs @@ -558,5 +558,17 @@ defmodule Livebook.LiveMarkdown.ImportTest do ] } = notebook end + + test "imports notebook :persist_outputs attribute" do + markdown = """ + + + # My Notebook + """ + + {notebook, []} = Import.notebook_from_markdown(markdown) + + assert %Notebook{name: "My Notebook", persist_outputs: true} = notebook + end end end diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index 68a3b619d..55243e769 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -48,6 +48,29 @@ defmodule Livebook.Session.DataTest do end end + describe "apply_operation/2 given :set_notebook_attributes" do + test "returns an error given an unknown attribute key" do + data = Data.new() + + attrs = %{unknown: :value} + operation = {:set_notebook_attributes, self(), attrs} + + assert :error = Data.apply_operation(data, operation) + end + + test "updates notebook with the given attributes" do + data = Data.new() + + attrs = %{persist_outputs: true} + operation = {:set_notebook_attributes, self(), attrs} + + assert {:ok, + %{ + notebook: %{persist_outputs: true} + }, _} = Data.apply_operation(data, operation) + end + end + describe "apply_operation/2 given :insert_section_into" do test "returns an error given invalid section id" do data = Data.new() @@ -1844,6 +1867,22 @@ defmodule Livebook.Session.DataTest do } }, []} = Data.apply_operation(data, operation) end + + test "sets dirty flag to true if outputs are persisted" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:set_runtime, self(), NoopRuntime.new()}, + {:queue_cell_evaluation, self(), "c1"}, + {:set_notebook_attributes, self(), %{persist_outputs: true}}, + {:mark_as_not_dirty, self()} + ]) + + operation = {:add_cell_evaluation_output, self(), "c1", "Hello!"} + + assert {:ok, %{dirty: true}, []} = Data.apply_operation(data, operation) + end end describe "apply_operation/2 given :add_cell_evaluation_response" do @@ -2055,6 +2094,23 @@ defmodule Livebook.Session.DataTest do assert evaluation_time >= 10 end + + test "sets dirty flag to true if outputs are persisted" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:set_runtime, self(), NoopRuntime.new()}, + {:queue_cell_evaluation, self(), "c1"}, + {:set_notebook_attributes, self(), %{persist_outputs: true}}, + {:mark_as_not_dirty, self()} + ]) + + operation = + {:add_cell_evaluation_response, self(), "c1", {:ok, [1, 2, 3]}, %{evaluation_time_ms: 10}} + + assert {:ok, %{dirty: true}, []} = Data.apply_operation(data, operation) + end end describe "apply_operation/2 given :bind_input" do diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs index 80af863d5..9f9462810 100644 --- a/test/livebook/session_test.exs +++ b/test/livebook/session_test.exs @@ -14,6 +14,17 @@ defmodule Livebook.SessionTest do %{session_id: session_id} end + describe "set_notebook_attributes/2" do + test "sends an attributes update to subscribers", %{session_id: session_id} do + Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") + pid = self() + + attrs = %{set_notebook_attributes: true} + Session.set_notebook_attributes(session_id, attrs) + assert_receive {:operation, {:set_notebook_attributes, ^pid, ^attrs}} + end + end + describe "insert_section/2" do test "sends an insert opreation to subscribers", %{session_id: session_id} do Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session_id}") diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index 25cf2d824..fc94a7e14 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -217,7 +217,7 @@ defmodule LivebookWeb.SessionLiveTest do |> render_click() view - |> element("form") + |> element(~s{form[phx-change="set_path"]}) |> render_change(%{path: path}) view @@ -228,6 +228,24 @@ defmodule LivebookWeb.SessionLiveTest do |> element("button", "notebook.livemd") |> has_element?() end + + test "changing output persistence updates data", %{conn: conn, session_id: session_id} do + {:ok, view, _} = live(conn, "/sessions/#{session_id}/settings/file") + + view + |> element("button", "Save to file") + |> render_click() + + view + |> element(~s{form[phx-change="set_options"]}) + |> render_change(%{persist_outputs: "true"}) + + view + |> element(~s{button[phx-click="save"]}, "Save") + |> render_click() + + assert %{notebook: %{persist_outputs: true}} = Session.get_data(session_id) + end end describe "completion" do