diff --git a/lib/livebook/live_markdown/export.ex b/lib/livebook/live_markdown/export.ex index 65be5f358..afe33278e 100644 --- a/lib/livebook/live_markdown/export.ex +++ b/lib/livebook/live_markdown/export.ex @@ -106,7 +106,7 @@ defmodule Livebook.LiveMarkdown.Export do json = %{ livebook_object: :cell_input, - type: cell.type, + type: Cell.Input.type_to_string(cell.type), name: cell.name, value: value } diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex index 9e567b5a1..8bc50a40d 100644 --- a/lib/livebook/live_markdown/import.ex +++ b/lib/livebook/live_markdown/import.ex @@ -14,14 +14,11 @@ defmodule Livebook.LiveMarkdown.Import do earmark_messages = Enum.map(earmark_messages, &earmark_message_to_string/1) {ast, rewrite_messages} = rewrite_ast(ast) + elements = group_elements(ast) + {notebook, build_messages} = build_notebook(elements) + notebook = postprocess_notebook(notebook) - notebook = - ast - |> group_elements() - |> build_notebook() - |> postprocess_notebook() - - {notebook, earmark_messages ++ rewrite_messages} + {notebook, earmark_messages ++ rewrite_messages ++ build_messages} end defp earmark_message_to_string({_severity, line_number, message}) do @@ -196,61 +193,68 @@ defmodule Livebook.LiveMarkdown.Import do # Note that the list of elements is reversed: # first we group elements by traversing Earmark AST top-down # and then aggregate elements into data strictures going bottom-up. - defp build_notebook(elems, cells \\ [], sections \\ []) + defp build_notebook(elems, cells \\ [], sections \\ [], messages \\ []) - defp build_notebook([{:cell, :elixir, source, outputs} | elems], cells, sections) do + defp build_notebook([{:cell, :elixir, source, outputs} | elems], cells, sections, messages) do {metadata, elems} = grab_metadata(elems) attrs = cell_metadata_to_attrs(:elixir, metadata) outputs = Enum.map(outputs, &{:text, &1}) cell = %{Notebook.Cell.new(:elixir) | source: source, outputs: outputs} |> Map.merge(attrs) - build_notebook(elems, [cell | cells], sections) + build_notebook(elems, [cell | cells], sections, messages) end - defp build_notebook([{:cell, :markdown, md_ast} | elems], cells, sections) do + defp build_notebook([{:cell, :markdown, md_ast} | elems], cells, sections, messages) do {metadata, elems} = grab_metadata(elems) attrs = cell_metadata_to_attrs(:markdown, metadata) source = md_ast |> Enum.reverse() |> MarkdownHelpers.markdown_from_ast() cell = %{Notebook.Cell.new(:markdown) | source: source} |> Map.merge(attrs) - build_notebook(elems, [cell | cells], sections) + build_notebook(elems, [cell | cells], sections, messages) end - defp build_notebook([{:cell, :input, data} | elems], cells, sections) do - attrs = parse_input_attrs(data) - cell = Notebook.Cell.new(:input) |> Map.merge(attrs) - build_notebook(elems, [cell | cells], sections) + defp build_notebook([{:cell, :input, data} | elems], cells, sections, messages) do + case parse_input_attrs(data) do + {:ok, attrs} -> + cell = Notebook.Cell.new(:input) |> Map.merge(attrs) + build_notebook(elems, [cell | cells], sections, messages) + + {:error, message} -> + build_notebook(elems, cells, sections, [message | messages]) + end end - defp build_notebook([{:section_name, content} | elems], cells, sections) do + defp build_notebook([{:section_name, content} | elems], cells, sections, messages) do name = text_from_markdown(content) {metadata, elems} = grab_metadata(elems) attrs = section_metadata_to_attrs(metadata) section = %{Notebook.Section.new() | name: name, cells: cells} |> Map.merge(attrs) - build_notebook(elems, [], [section | sections]) + build_notebook(elems, [], [section | sections], messages) end # If there are section-less cells, put them in a default one. - defp build_notebook([{:notebook_name, _content} | _] = elems, cells, sections) + defp build_notebook([{:notebook_name, _content} | _] = elems, cells, sections, messages) when cells != [] do section = %{Notebook.Section.new() | cells: cells} - build_notebook(elems, [], [section | sections]) + build_notebook(elems, [], [section | sections], messages) end # If there are section-less cells, put them in a default one. - defp build_notebook([] = elems, cells, sections) when cells != [] do + defp build_notebook([] = elems, cells, sections, messages) when cells != [] do section = %{Notebook.Section.new() | cells: cells} - build_notebook(elems, [], [section | sections]) + build_notebook(elems, [], [section | sections], messages) end - defp build_notebook([{:notebook_name, content} | elems], [], sections) do + defp build_notebook([{:notebook_name, content} | elems], [], sections, messages) do name = text_from_markdown(content) {metadata, []} = grab_metadata(elems) attrs = notebook_metadata_to_attrs(metadata) - %{Notebook.new() | name: name, sections: sections} |> Map.merge(attrs) + notebook = %{Notebook.new() | name: name, sections: sections} |> Map.merge(attrs) + {notebook, messages} end # If there's no explicit notebook heading, use the defaults. - defp build_notebook([], [], sections) do - %{Notebook.new() | sections: sections} + defp build_notebook([], [], sections, messages) do + notebook = %{Notebook.new() | sections: sections} + {notebook, messages} end defp text_from_markdown(markdown) do @@ -268,16 +272,28 @@ defmodule Livebook.LiveMarkdown.Import do defp grab_metadata(elems), do: {%{}, elems} defp parse_input_attrs(data) do - type = data["type"] |> String.to_existing_atom() + with {:ok, type} <- parse_input_type(data["type"]) do + {:ok, + %{ + type: type, + name: data["name"], + value: data["value"], + # Fields with implicit value + reactive: Map.get(data, "reactive", false), + props: data |> Map.get("props", %{}) |> parse_input_props(type) + }} + end + end - %{ - type: type, - name: data["name"], - value: data["value"], - # Fields with implicit value - reactive: Map.get(data, "reactive", false), - props: data |> Map.get("props", %{}) |> parse_input_props(type) - } + defp parse_input_type(string) do + case Notebook.Cell.Input.type_from_string(string) do + {:ok, type} -> + {:ok, type} + + :error -> + {:error, + "unrecognised input type #{inspect(string)}, if it's a valid type it means your Livebook version doesn't support it"} + end end defp parse_input_props(data, type) do diff --git a/lib/livebook/notebook/cell/input.ex b/lib/livebook/notebook/cell/input.ex index 29c131b25..25864413a 100644 --- a/lib/livebook/notebook/cell/input.ex +++ b/lib/livebook/notebook/cell/input.ex @@ -20,6 +20,7 @@ defmodule Livebook.Notebook.Cell.Input do props: props() } + # Make sure to keep this in sync with `type_from_string/1` @type type :: :text | :url | :number | :password | :textarea | :color | :range | :select | :checkbox @@ -114,4 +115,31 @@ defmodule Livebook.Notebook.Cell.Input do def reactive_update?(cell, prev_cell) do cell.reactive and cell.value != prev_cell.value and validate(cell) == :ok end + + @doc """ + Parses input type from string. + """ + @spec type_from_string(String.t()) :: {:ok, type()} | :error + def type_from_string(string) do + case string do + "text" -> {:ok, :text} + "url" -> {:ok, :url} + "number" -> {:ok, :number} + "password" -> {:ok, :password} + "textarea" -> {:ok, :textarea} + "color" -> {:ok, :color} + "range" -> {:ok, :range} + "select" -> {:ok, :select} + "checkbox" -> {:ok, :checkbox} + _other -> :error + end + end + + @doc """ + Converts inpu type to string. + """ + @spec type_to_string(type()) :: String.t() + def type_to_string(type) do + Atom.to_string(type) + end end diff --git a/test/livebook/live_markdown/import_test.exs b/test/livebook/live_markdown/import_test.exs index b563d11df..13a390b15 100644 --- a/test/livebook/live_markdown/import_test.exs +++ b/test/livebook/live_markdown/import_test.exs @@ -581,4 +581,20 @@ defmodule Livebook.LiveMarkdown.ImportTest do assert %Notebook{name: "My Notebook", autosave_interval_s: 10} = notebook end + + test "skips invalid input type and returns a message" do + markdown = """ + # My Notebook + + ## Section 1 + + + """ + + {_notebook, messages} = Import.notebook_from_markdown(markdown) + + assert [ + ~s{unrecognised input type "input_from_the_future", if it's a valid type it means your Livebook version doesn't support it} + ] == messages + end end