Add notebook option for persisting outputs (#485)

* Add notebook option for persisting outputs

* Increase persistence modal spacing

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

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

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
 

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()}

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.
   """

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),

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

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"""
-    <div class="p-6 pb-4 flex flex-col space-y-3">
+    <div class="p-6 pb-4 flex flex-col space-y-8">
       <h3 class="text-2xl font-semibold text-gray-800">
         File
       </h3>
-      <div class="w-full flex-col space-y-5">
-        <p class="text-gray-700">
-          Specify where the notebook should be automatically persisted.
-        </p>
+      <div class="w-full flex-col space-y-8">
+        <div class="flex">
+          <form phx-change="set_options" onsubmit="return false;" phx-target={@myself}>
+            <.switch_checkbox
+              name="persist_outputs"
+              label="Persist outputs"
+              checked={@new_attrs.persist_outputs} />
+          </form>
+        </div>
         <div class="flex space-x-4">
           <.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>
           <.choice_button
-            active={@path == nil}
+            active={@new_attrs.path == nil}
             phx-click="set_persistence_type"
             phx-value-type="memory"
             phx-target={@myself}>
             Memory only
           </.choice_button>
         </div>
-        <%= if @path != nil do %>
+        <%= if @new_attrs.path != nil do %>
           <div class="h-full h-52">
             <%= 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") %>
           </div>
         <% end %>
-        <div class="flex flex-col space-y-2">
-          <%= if @path != nil do %>
+        <div class="flex flex-col space-y-8">
+          <%= if @new_attrs.path != nil do %>
             <div class="text-gray-500 text-sm">
-              File: <%= normalize_path(@path) %>
+              File: <%= normalize_path(@new_attrs.path) %>
             </div>
           <% end %>
           <div>
-            <button class="button button-blue mt-2"
+            <button class="button button-blue"
               phx-click="save"
               phx-target={@myself}
-              disabled={disabled?(@path, @current_path, @running_paths)}>
+              disabled={disabled?(@new_attrs, @attrs, @running_paths)}>
               Save
             </button>
           </div>
@@ -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

notebook.livemd

@@ -0,0 +1,3 @@
+<!-- livebook:{"persist_outputs":true} -->
+
+# Untitled notebook

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 = """
+    <!-- livebook:{"persist_outputs":true} -->
+
+    # 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 = """
+    <!-- livebook:{"persist_outputs":true} -->
+
+    # My Notebook
+
+    ## Section 1
+
+    ```elixir
+    IO.puts("hey")
+    ```
+    """
+
+    document = Export.notebook_to_markdown(notebook, include_outputs: false)
+
+    assert expected_document == document
+  end
 end

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 = """
+      <!-- livebook:{"persist_outputs":true} -->
+
+      # My Notebook
+      """
+
+      {notebook, []} = Import.notebook_from_markdown(markdown)
+
+      assert %Notebook{name: "My Notebook", persist_outputs: true} = notebook
+    end
   end
 end

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

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}")

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