Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not format code cells on save #2605

Merged
merged 2 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/livebook/live_markdown.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ defmodule Livebook.LiveMarkdown do
# * Elixir
# * PostgreSQL
#
# <!-- livebook:{"disable_formatting":true} -->
# <!-- livebook:{"reevaluate_automatically":true} -->
#
# ```elixir
# Enum.to_list(1..10)
Expand Down
17 changes: 2 additions & 15 deletions lib/livebook/live_markdown/export.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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, " -->"]
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions lib/livebook/live_markdown/import.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 0 additions & 3 deletions lib/livebook/notebook/cell/code.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ defmodule Livebook.Notebook.Cell.Code do
:source,
:outputs,
:language,
:disable_formatting,
:reevaluate_automatically,
:continue_on_error
]
Expand All @@ -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()
}
Expand All @@ -37,7 +35,6 @@ defmodule Livebook.Notebook.Cell.Code do
source: "",
outputs: [],
language: :elixir,
disable_formatting: false,
reevaluate_automatically: false,
continue_on_error: false
}
Expand Down
23 changes: 3 additions & 20 deletions lib/livebook/notebook/export/elixir.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -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
11 changes: 0 additions & 11 deletions lib/livebook_web/live/session_live/code_cell_settings_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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"
Expand Down Expand Up @@ -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
})
Expand Down
83 changes: 4 additions & 79 deletions test/livebook/live_markdown/export_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)\
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -1350,7 +1275,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
%{
Notebook.Cell.new(:code)
| source: """
IO.puts("hey")
IO.puts("hey")\
"""
}
]
Expand Down Expand Up @@ -1388,7 +1313,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
%{
Notebook.Cell.new(:code)
| source: """
IO.puts("hey")
IO.puts("hey")\
"""
}
]
Expand Down
3 changes: 1 addition & 2 deletions test/livebook/live_markdown/import_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -84,7 +84,6 @@ defmodule Livebook.LiveMarkdown.ImportTest do
"""
},
%Cell.Code{
disable_formatting: true,
reevaluate_automatically: true,
continue_on_error: true,
source: """
Expand Down
3 changes: 1 addition & 2 deletions test/livebook/notebook/export/elixir_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ defmodule Livebook.Notebook.Export.ElixirTest do
},
%{
Notebook.Cell.new(:code)
| disable_formatting: true,
source: """
| source: """
Enum.to_list(1..10)\
"""
},
Expand Down
4 changes: 2 additions & 2 deletions test/livebook/session/data_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/livebook/session_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
Loading