diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex index 9a337c306..a276ac82e 100644 --- a/lib/livebook/live_markdown/import.ex +++ b/lib/livebook/live_markdown/import.ex @@ -16,9 +16,9 @@ defmodule Livebook.LiveMarkdown.Import do {ast, rewrite_messages} = rewrite_ast(ast) elements = group_elements(ast) {notebook, build_messages} = build_notebook(elements) - notebook = postprocess_notebook(notebook) + {notebook, postprocess_messages} = postprocess_notebook(notebook) - {notebook, earmark_messages ++ rewrite_messages ++ build_messages} + {notebook, earmark_messages ++ rewrite_messages ++ build_messages ++ postprocess_messages} end defp earmark_message_to_string({_severity, line_number, message}) do @@ -393,19 +393,47 @@ defmodule Livebook.LiveMarkdown.Import do end defp postprocess_notebook(notebook) do - sections = - Enum.map(notebook.sections, fn section -> + {sections, {_branching_ids, warnings}} = + notebook.sections + |> Enum.with_index() + |> Enum.map_reduce({MapSet.new(), []}, fn {section, section_idx}, + {branching_ids, warnings} -> # Set parent_id based on the persisted branch_parent_index if present case section.parent_id do nil -> - section + {section, {branching_ids, warnings}} {:idx, parent_idx} -> parent = Enum.at(notebook.sections, parent_idx) - %{section | parent_id: parent.id} + + cond do + section_idx <= parent_idx -> + {%{section | parent_id: nil}, + { + branching_ids, + [ + ~s{ignoring the parent section of "#{section.name}", because it comes later in the notebook} + | warnings + ] + }} + + !is_nil(parent) && MapSet.member?(branching_ids, parent.id) -> + {%{section | parent_id: nil}, + { + branching_ids, + [ + ~s{ignoring the parent section of "#{section.name}", because it is itself a branching section} + | warnings + ] + }} + + true -> + {%{section | parent_id: parent.id}, + {MapSet.put(branching_ids, section.id), warnings}} + end end end) - %{notebook | sections: sections} + {%{notebook | sections: sections}, Enum.reverse(warnings)} end end diff --git a/test/livebook/live_markdown/import_test.exs b/test/livebook/live_markdown/import_test.exs index b03fb2d22..795a0ac9d 100644 --- a/test/livebook/live_markdown/import_test.exs +++ b/test/livebook/live_markdown/import_test.exs @@ -766,4 +766,120 @@ defmodule Livebook.LiveMarkdown.ImportTest do } = notebook end end + + test "import notebook with invalid parent section produces a warning" do + markdown = """ + # My Notebook + + + + ## Section 1 + + ```elixir + Process.info() + ``` + + ## Section 2 + + ```elixir + Process.info() + ``` + """ + + assert {notebook, + [ + "ignoring the parent section of \"Section 1\", because it comes later in the notebook" + ]} = Import.notebook_from_markdown(markdown) + + assert %Notebook{ + name: "My Notebook", + sections: [ + %Notebook.Section{ + parent_id: nil + }, + %Notebook.Section{ + parent_id: nil + } + ] + } = notebook + end + + test "import notebook with parent section pointing to the section itself produces a warning" do + markdown = """ + # My Notebook + + + + ## Section 1 + + ```elixir + Process.info() + ``` + """ + + assert {notebook, + [ + "ignoring the parent section of \"Section 1\", because it comes later in the notebook" + ]} = Import.notebook_from_markdown(markdown) + + assert %Notebook{ + name: "My Notebook", + sections: [ + %Notebook.Section{ + parent_id: nil + } + ] + } = notebook + end + + test "import notebook with parent section being a branching section itself produces a warning" do + markdown = """ + # My Notebook + + + ## Section 1 + + ```elixir + Process.info() + ``` + + + + ## Section 2 + + ```elixir + Process.info() + ``` + + + ## Section 3 + + ```elixir + Process.info() + ``` + """ + + assert {notebook, + [ + "ignoring the parent section of \"Section 3\", because it is itself a branching section" + ]} = Import.notebook_from_markdown(markdown) + + assert %Notebook{ + name: "My Notebook", + sections: [ + %Notebook.Section{ + name: "Section 1", + parent_id: nil + }, + %Notebook.Section{ + name: "Section 2", + parent_id: _ + }, + %Notebook.Section{ + name: "Section 3", + parent_id: nil + } + ] + } = notebook + end end