Do not format code cells on save (#2605)

livebook-dev · May 14, 2024 · c0b85ac · c0b85ac
1 parent 6f1e09e
commit c0b85ac
Show file tree

Hide file tree

Showing 11 changed files with 15 additions and 139 deletions.
diff --git a/lib/livebook/live_markdown.ex b/lib/livebook/live_markdown.ex
@@ -65,7 +65,7 @@ defmodule Livebook.LiveMarkdown do
   #     * Elixir
   #     * PostgreSQL
   #
-  #     <!-- livebook:{"disable_formatting":true} -->
+  #     <!-- livebook:{"reevaluate_automatically":true} -->
   #
   #     ```elixir
   #     Enum.to_list(1..10)

diff --git a/lib/livebook/live_markdown/export.ex b/lib/livebook/live_markdown/export.ex
@@ -210,7 +210,7 @@ defmodule Livebook.LiveMarkdown.Export do
 
   defp render_cell(%Cell.Code{} = cell, ctx) do
     delimiter = MarkdownHelpers.code_block_delimiter(cell.source)
-    code = get_code_cell_code(cell)
+    code = cell.source
     outputs = if ctx.include_outputs?, do: render_outputs(cell, ctx), else: []
 
     metadata = cell_metadata(cell)
@@ -240,7 +240,7 @@ defmodule Livebook.LiveMarkdown.Export do
   end
 
   defp cell_metadata(%Cell.Code{} = cell) do
-    keys = [:disable_formatting, :reevaluate_automatically, :continue_on_error]
+    keys = [:reevaluate_automatically, :continue_on_error]
     put_unless_default(%{}, Map.take(cell, keys), Map.take(Cell.Code.new(), keys))
   end
 
@@ -299,11 +299,6 @@ defmodule Livebook.LiveMarkdown.Export do
   defp encode_js_data(data) when is_binary(data), do: {:ok, data}
   defp encode_js_data(data), do: data |> ensure_order() |> Jason.encode()
 
-  defp get_code_cell_code(%{source: source, language: :elixir, disable_formatting: false}),
-    do: format_elixir_code(source)
-
-  defp get_code_cell_code(%{source: source}), do: source
-
   defp render_metadata(metadata) do
     metadata_json = metadata |> ensure_order() |> Jason.encode!()
     ["<!-- livebook:", metadata_json, " -->"]
@@ -349,14 +344,6 @@ defmodule Livebook.LiveMarkdown.Export do
     end)
   end
 
-  defp format_elixir_code(code) do
-    try do
-      Code.format_string!(code)
-    rescue
-      _ -> code
-    end
-  end
-
   defp put_unless_default(map, entries, defaults) do
     Enum.reduce(entries, map, fn {key, value}, map ->
       if value == defaults[key] do

diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex
@@ -537,9 +537,6 @@ defmodule Livebook.LiveMarkdown.Import do
 
   defp cell_metadata_to_attrs(:code, metadata) do
     Enum.reduce(metadata, %{}, fn
-      {"disable_formatting", disable_formatting}, attrs ->
-        Map.put(attrs, :disable_formatting, disable_formatting)
-
       {"reevaluate_automatically", reevaluate_automatically}, attrs ->
         Map.put(attrs, :reevaluate_automatically, reevaluate_automatically)
 

diff --git a/lib/livebook/notebook/cell/code.ex b/lib/livebook/notebook/cell/code.ex
@@ -9,7 +9,6 @@ defmodule Livebook.Notebook.Cell.Code do
     :source,
     :outputs,
     :language,
-    :disable_formatting,
     :reevaluate_automatically,
     :continue_on_error
   ]
@@ -22,7 +21,6 @@ defmodule Livebook.Notebook.Cell.Code do
           source: String.t() | :__pruned__,
           outputs: list(Cell.indexed_output()),
           language: :elixir | :erlang,
-          disable_formatting: boolean(),
           reevaluate_automatically: boolean(),
           continue_on_error: boolean()
         }
@@ -37,7 +35,6 @@ defmodule Livebook.Notebook.Cell.Code do
       source: "",
       outputs: [],
       language: :elixir,
-      disable_formatting: false,
       reevaluate_automatically: false,
       continue_on_error: false
     }

diff --git a/lib/livebook/notebook/export/elixir.ex b/lib/livebook/notebook/export/elixir.ex
@@ -40,7 +40,7 @@ defmodule Livebook.Notebook.Export.Elixir do
     cells =
       section.cells
       |> Enum.map(&render_cell(&1, section))
-      |> Enum.reject(&(&1 == []))
+      |> Enum.reject(&(&1 in ["", []]))
 
     [name | cells]
     |> Enum.intersperse("\n\n")
@@ -57,23 +57,19 @@ defmodule Livebook.Notebook.Export.Elixir do
   end
 
   defp render_cell(%Cell.Code{language: :elixir} = cell, section) do
-    code = get_code_cell_code(cell)
-
     if section.parent_id do
-      code
-      |> IO.iodata_to_binary()
+      cell.source
       |> String.split("\n")
       |> Enum.map_intersperse("\n", &comment_out/1)
     else
-      code
+      cell.source
     end
   end
 
   defp render_cell(%Cell.Code{} = cell, _section) do
     code = cell.source
 
     code
-    |> IO.iodata_to_binary()
     |> String.split("\n")
     |> Enum.map_intersperse("\n", &comment_out/1)
   end
@@ -86,17 +82,4 @@ defmodule Livebook.Notebook.Export.Elixir do
 
   defp comment_out(""), do: ""
   defp comment_out(line), do: ["# ", line]
-
-  defp get_code_cell_code(%{source: source, disable_formatting: true}),
-    do: source
-
-  defp get_code_cell_code(%{source: source}), do: format_code(source)
-
-  defp format_code(code) do
-    try do
-      Code.format_string!(code)
-    rescue
-      _ -> code
-    end
-  end
 end
diff --git a/lib/livebook_web/live/session_live/code_cell_settings_component.ex b/lib/livebook_web/live/session_live/code_cell_settings_component.ex
@@ -10,7 +10,6 @@ defmodule LivebookWeb.SessionLive.CodeCellSettingsComponent do
     socket =
       socket
       |> assign(assigns)
-      |> assign_new(:disable_formatting, fn -> cell.disable_formatting end)
       |> assign_new(:reevaluate_automatically, fn -> cell.reevaluate_automatically end)
       |> assign_new(:continue_on_error, fn -> cell.continue_on_error end)
 
@@ -25,13 +24,6 @@ defmodule LivebookWeb.SessionLive.CodeCellSettingsComponent do
         Cell settings
       </h3>
       <form phx-submit="save" phx-target={@myself}>
-        <div :if={@cell.language == :elixir} class="w-full flex-col space-y-6">
-          <.switch_field
-            name="enable_formatting"
-            label="Format code when saving to file"
-            value={not @disable_formatting}
-          />
-        </div>
         <div class="w-full flex-col space-y-6 mt-4">
           <.switch_field
             name="reevaluate_automatically"
@@ -63,18 +55,15 @@ defmodule LivebookWeb.SessionLive.CodeCellSettingsComponent do
   def handle_event(
         "save",
         %{
-          "enable_formatting" => enable_formatting,
           "reevaluate_automatically" => reevaluate_automatically,
           "continue_on_error" => continue_on_error
         },
         socket
       ) do
-    disable_formatting = enable_formatting == "false"
     reevaluate_automatically = reevaluate_automatically == "true"
     continue_on_error = continue_on_error == "true"
 
     Session.set_cell_attributes(socket.assigns.session.pid, socket.assigns.cell.id, %{
-      disable_formatting: disable_formatting,
       reevaluate_automatically: reevaluate_automatically,
       continue_on_error: continue_on_error
     })

diff --git a/test/livebook/live_markdown/export_test.exs b/test/livebook/live_markdown/export_test.exs
@@ -29,8 +29,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
                 },
                 %{
                   Notebook.Cell.new(:code)
-                  | disable_formatting: true,
-                    reevaluate_automatically: true,
+                  | reevaluate_automatically: true,
                     continue_on_error: true,
                     source: """
                     Enum.to_list(1..10)\
@@ -111,7 +110,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
 
     $x_{i} + y_{i}$
 
-    <!-- livebook:{"continue_on_error":true,"disable_formatting":true,"reevaluate_automatically":true} -->
+    <!-- livebook:{"continue_on_error":true,"reevaluate_automatically":true} -->
 
     ```elixir
     Enum.to_list(1..10)
@@ -343,80 +342,6 @@ defmodule Livebook.LiveMarkdown.ExportTest do
     assert expected_document == document
   end
 
-  test "formats code in code cells" do
-    notebook = %{
-      Notebook.new()
-      | name: "My Notebook",
-        sections: [
-          %{
-            Notebook.Section.new()
-            | name: "Section 1",
-              cells: [
-                %{
-                  Notebook.Cell.new(:code)
-                  | source: """
-                    [1,2,3] # Comment
-                    """
-                }
-              ]
-          }
-        ]
-    }
-
-    expected_document = """
-    # My Notebook
-
-    ## Section 1
-
-    ```elixir
-    # Comment
-    [1, 2, 3]
-    ```
-    """
-
-    {document, []} = Export.notebook_to_livemd(notebook)
-
-    assert expected_document == document
-  end
-
-  test "does not format code in code cells which have formatting disabled" do
-    notebook = %{
-      Notebook.new()
-      | name: "My Notebook",
-        sections: [
-          %{
-            Notebook.Section.new()
-            | name: "Section 1",
-              cells: [
-                %{
-                  Notebook.Cell.new(:code)
-                  | disable_formatting: true,
-                    source: """
-                    [1,2,3] # Comment\
-                    """
-                }
-              ]
-          }
-        ]
-    }
-
-    expected_document = """
-    # My Notebook
-
-    ## Section 1
-
-    <!-- livebook:{"disable_formatting":true} -->
-
-    ```elixir
-    [1,2,3] # Comment
-    ```
-    """
-
-    {document, []} = Export.notebook_to_livemd(notebook)
-
-    assert expected_document == document
-  end
-
   test "handles backticks in code cell" do
     notebook = %{
       Notebook.new()
@@ -1350,7 +1275,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
                   %{
                     Notebook.Cell.new(:code)
                     | source: """
-                      IO.puts("hey")
+                      IO.puts("hey")\
                       """
                   }
                 ]
@@ -1388,7 +1313,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
                   %{
                     Notebook.Cell.new(:code)
                     | source: """
-                      IO.puts("hey")
+                      IO.puts("hey")\
                       """
                   }
                 ]

diff --git a/test/livebook/live_markdown/import_test.exs b/test/livebook/live_markdown/import_test.exs
@@ -21,7 +21,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
 
     $x_{i} + y_{i}$
 
-    <!-- livebook:{"continue_on_error":true,"disable_formatting":true,"reevaluate_automatically":true} -->
+    <!-- livebook:{"continue_on_error":true,"reevaluate_automatically":true} -->
 
     ```elixir
     Enum.to_list(1..10)
@@ -84,7 +84,6 @@ defmodule Livebook.LiveMarkdown.ImportTest do
                      """
                    },
                    %Cell.Code{
-                     disable_formatting: true,
                      reevaluate_automatically: true,
                      continue_on_error: true,
                      source: """

diff --git a/test/livebook/notebook/export/elixir_test.exs b/test/livebook/notebook/export/elixir_test.exs
@@ -25,8 +25,7 @@ defmodule Livebook.Notebook.Export.ElixirTest do
                 },
                 %{
                   Notebook.Cell.new(:code)
-                  | disable_formatting: true,
-                    source: """
+                  | source: """
                     Enum.to_list(1..10)\
                     """
                 },

diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs
@@ -3672,14 +3672,14 @@ defmodule Livebook.Session.DataTest do
           {:insert_cell, @cid, "s1", 0, :code, "c1", %{}}
         ])
 
-      attrs = %{disable_formatting: true, reevaluate_automatically: true}
+      attrs = %{reevaluate_automatically: true}
       operation = {:set_cell_attributes, @cid, "c1", attrs}
 
       assert {:ok,
               %{
                 notebook: %{
                   sections: [
-                    %{cells: [%{disable_formatting: true, reevaluate_automatically: true}]}
+                    %{cells: [%{reevaluate_automatically: true}]}
                   ]
                 }
               }, _} = Data.apply_operation(data, operation)

diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs
@@ -379,7 +379,7 @@ defmodule Livebook.SessionTest do
       Session.subscribe(session.id)
 
       {_section_id, cell_id} = insert_section_and_cell(session.pid)
-      attrs = %{disable_formatting: true}
+      attrs = %{reevaluate_automatically: true}
 
       Session.set_cell_attributes(session.pid, cell_id, attrs)
       assert_receive {:operation, {:set_cell_attributes, _client_id, ^cell_id, ^attrs}}