Do not format code cells on save (#2605)

This commit is contained in:
Jonatan Kłosko 2024-05-14 17:33:07 +02:00 committed by GitHub
parent 6f1e09e09b
commit c0b85acb0a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 15 additions and 139 deletions

View file

@ -65,7 +65,7 @@ defmodule Livebook.LiveMarkdown do
# * Elixir # * Elixir
# * PostgreSQL # * PostgreSQL
# #
# <!-- livebook:{"disable_formatting":true} --> # <!-- livebook:{"reevaluate_automatically":true} -->
# #
# ```elixir # ```elixir
# Enum.to_list(1..10) # Enum.to_list(1..10)

View file

@ -210,7 +210,7 @@ defmodule Livebook.LiveMarkdown.Export do
defp render_cell(%Cell.Code{} = cell, ctx) do defp render_cell(%Cell.Code{} = cell, ctx) do
delimiter = MarkdownHelpers.code_block_delimiter(cell.source) 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: [] outputs = if ctx.include_outputs?, do: render_outputs(cell, ctx), else: []
metadata = cell_metadata(cell) metadata = cell_metadata(cell)
@ -240,7 +240,7 @@ defmodule Livebook.LiveMarkdown.Export do
end end
defp cell_metadata(%Cell.Code{} = cell) do 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)) put_unless_default(%{}, Map.take(cell, keys), Map.take(Cell.Code.new(), keys))
end 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) when is_binary(data), do: {:ok, data}
defp encode_js_data(data), do: data |> ensure_order() |> Jason.encode() 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 defp render_metadata(metadata) do
metadata_json = metadata |> ensure_order() |> Jason.encode!() metadata_json = metadata |> ensure_order() |> Jason.encode!()
["<!-- livebook:", metadata_json, " -->"] ["<!-- livebook:", metadata_json, " -->"]
@ -349,14 +344,6 @@ defmodule Livebook.LiveMarkdown.Export do
end) end)
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 defp put_unless_default(map, entries, defaults) do
Enum.reduce(entries, map, fn {key, value}, map -> Enum.reduce(entries, map, fn {key, value}, map ->
if value == defaults[key] do if value == defaults[key] do

View file

@ -537,9 +537,6 @@ defmodule Livebook.LiveMarkdown.Import do
defp cell_metadata_to_attrs(:code, metadata) do defp cell_metadata_to_attrs(:code, metadata) do
Enum.reduce(metadata, %{}, fn Enum.reduce(metadata, %{}, fn
{"disable_formatting", disable_formatting}, attrs ->
Map.put(attrs, :disable_formatting, disable_formatting)
{"reevaluate_automatically", reevaluate_automatically}, attrs -> {"reevaluate_automatically", reevaluate_automatically}, attrs ->
Map.put(attrs, :reevaluate_automatically, reevaluate_automatically) Map.put(attrs, :reevaluate_automatically, reevaluate_automatically)

View file

@ -9,7 +9,6 @@ defmodule Livebook.Notebook.Cell.Code do
:source, :source,
:outputs, :outputs,
:language, :language,
:disable_formatting,
:reevaluate_automatically, :reevaluate_automatically,
:continue_on_error :continue_on_error
] ]
@ -22,7 +21,6 @@ defmodule Livebook.Notebook.Cell.Code do
source: String.t() | :__pruned__, source: String.t() | :__pruned__,
outputs: list(Cell.indexed_output()), outputs: list(Cell.indexed_output()),
language: :elixir | :erlang, language: :elixir | :erlang,
disable_formatting: boolean(),
reevaluate_automatically: boolean(), reevaluate_automatically: boolean(),
continue_on_error: boolean() continue_on_error: boolean()
} }
@ -37,7 +35,6 @@ defmodule Livebook.Notebook.Cell.Code do
source: "", source: "",
outputs: [], outputs: [],
language: :elixir, language: :elixir,
disable_formatting: false,
reevaluate_automatically: false, reevaluate_automatically: false,
continue_on_error: false continue_on_error: false
} }

View file

@ -40,7 +40,7 @@ defmodule Livebook.Notebook.Export.Elixir do
cells = cells =
section.cells section.cells
|> Enum.map(&render_cell(&1, section)) |> Enum.map(&render_cell(&1, section))
|> Enum.reject(&(&1 == [])) |> Enum.reject(&(&1 in ["", []]))
[name | cells] [name | cells]
|> Enum.intersperse("\n\n") |> Enum.intersperse("\n\n")
@ -57,15 +57,12 @@ defmodule Livebook.Notebook.Export.Elixir do
end end
defp render_cell(%Cell.Code{language: :elixir} = cell, section) do defp render_cell(%Cell.Code{language: :elixir} = cell, section) do
code = get_code_cell_code(cell)
if section.parent_id do if section.parent_id do
code cell.source
|> IO.iodata_to_binary()
|> String.split("\n") |> String.split("\n")
|> Enum.map_intersperse("\n", &comment_out/1) |> Enum.map_intersperse("\n", &comment_out/1)
else else
code cell.source
end end
end end
@ -73,7 +70,6 @@ defmodule Livebook.Notebook.Export.Elixir do
code = cell.source code = cell.source
code code
|> IO.iodata_to_binary()
|> String.split("\n") |> String.split("\n")
|> Enum.map_intersperse("\n", &comment_out/1) |> Enum.map_intersperse("\n", &comment_out/1)
end end
@ -86,17 +82,4 @@ defmodule Livebook.Notebook.Export.Elixir do
defp comment_out(""), do: "" defp comment_out(""), do: ""
defp comment_out(line), do: ["# ", line] 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 end

View file

@ -10,7 +10,6 @@ defmodule LivebookWeb.SessionLive.CodeCellSettingsComponent do
socket = socket =
socket socket
|> assign(assigns) |> assign(assigns)
|> assign_new(:disable_formatting, fn -> cell.disable_formatting end)
|> assign_new(:reevaluate_automatically, fn -> cell.reevaluate_automatically end) |> assign_new(:reevaluate_automatically, fn -> cell.reevaluate_automatically end)
|> assign_new(:continue_on_error, fn -> cell.continue_on_error end) |> assign_new(:continue_on_error, fn -> cell.continue_on_error end)
@ -25,13 +24,6 @@ defmodule LivebookWeb.SessionLive.CodeCellSettingsComponent do
Cell settings Cell settings
</h3> </h3>
<form phx-submit="save" phx-target={@myself}> <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"> <div class="w-full flex-col space-y-6 mt-4">
<.switch_field <.switch_field
name="reevaluate_automatically" name="reevaluate_automatically"
@ -63,18 +55,15 @@ defmodule LivebookWeb.SessionLive.CodeCellSettingsComponent do
def handle_event( def handle_event(
"save", "save",
%{ %{
"enable_formatting" => enable_formatting,
"reevaluate_automatically" => reevaluate_automatically, "reevaluate_automatically" => reevaluate_automatically,
"continue_on_error" => continue_on_error "continue_on_error" => continue_on_error
}, },
socket socket
) do ) do
disable_formatting = enable_formatting == "false"
reevaluate_automatically = reevaluate_automatically == "true" reevaluate_automatically = reevaluate_automatically == "true"
continue_on_error = continue_on_error == "true" continue_on_error = continue_on_error == "true"
Session.set_cell_attributes(socket.assigns.session.pid, socket.assigns.cell.id, %{ Session.set_cell_attributes(socket.assigns.session.pid, socket.assigns.cell.id, %{
disable_formatting: disable_formatting,
reevaluate_automatically: reevaluate_automatically, reevaluate_automatically: reevaluate_automatically,
continue_on_error: continue_on_error continue_on_error: continue_on_error
}) })

View file

@ -29,8 +29,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
}, },
%{ %{
Notebook.Cell.new(:code) Notebook.Cell.new(:code)
| disable_formatting: true, | reevaluate_automatically: true,
reevaluate_automatically: true,
continue_on_error: true, continue_on_error: true,
source: """ source: """
Enum.to_list(1..10)\ Enum.to_list(1..10)\
@ -111,7 +110,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
$x_{i} + y_{i}$ $x_{i} + y_{i}$
<!-- livebook:{"continue_on_error":true,"disable_formatting":true,"reevaluate_automatically":true} --> <!-- livebook:{"continue_on_error":true,"reevaluate_automatically":true} -->
```elixir ```elixir
Enum.to_list(1..10) Enum.to_list(1..10)
@ -343,80 +342,6 @@ defmodule Livebook.LiveMarkdown.ExportTest do
assert expected_document == document assert expected_document == document
end 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 test "handles backticks in code cell" do
notebook = %{ notebook = %{
Notebook.new() Notebook.new()
@ -1350,7 +1275,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
%{ %{
Notebook.Cell.new(:code) Notebook.Cell.new(:code)
| source: """ | source: """
IO.puts("hey") IO.puts("hey")\
""" """
} }
] ]
@ -1388,7 +1313,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
%{ %{
Notebook.Cell.new(:code) Notebook.Cell.new(:code)
| source: """ | source: """
IO.puts("hey") IO.puts("hey")\
""" """
} }
] ]

View file

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

View file

@ -25,8 +25,7 @@ defmodule Livebook.Notebook.Export.ElixirTest do
}, },
%{ %{
Notebook.Cell.new(:code) Notebook.Cell.new(:code)
| disable_formatting: true, | source: """
source: """
Enum.to_list(1..10)\ Enum.to_list(1..10)\
""" """
}, },

View file

@ -3672,14 +3672,14 @@ defmodule Livebook.Session.DataTest do
{:insert_cell, @cid, "s1", 0, :code, "c1", %{}} {: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} operation = {:set_cell_attributes, @cid, "c1", attrs}
assert {:ok, assert {:ok,
%{ %{
notebook: %{ notebook: %{
sections: [ sections: [
%{cells: [%{disable_formatting: true, reevaluate_automatically: true}]} %{cells: [%{reevaluate_automatically: true}]}
] ]
} }
}, _} = Data.apply_operation(data, operation) }, _} = Data.apply_operation(data, operation)

View file

@ -379,7 +379,7 @@ defmodule Livebook.SessionTest do
Session.subscribe(session.id) Session.subscribe(session.id)
{_section_id, cell_id} = insert_section_and_cell(session.pid) {_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) Session.set_cell_attributes(session.pid, cell_id, attrs)
assert_receive {:operation, {:set_cell_attributes, _client_id, ^cell_id, ^attrs}} assert_receive {:operation, {:set_cell_attributes, _client_id, ^cell_id, ^attrs}}