LibMarkdown: Change internal MD API to return OwnPtrs

Previously, all Markdown blocks had a virtual parse method which has
been swapped out for a static parse method returning an OwnPtr of
that block's type.

The Text class also now has a static parse method that will return an
Optional<Text>.

Applications/Help/main.cpp

@@ -128,10 +128,13 @@ int main(int argc, char* argv[])
         auto buffer = file->read_all();
         StringView source { (const char*)buffer.data(), buffer.size() };
 
-        auto md_document = Markdown::Document::parse(source);
+        String html;
-        ASSERT(md_document);
+        {
+            auto md_document = Markdown::Document::parse(source);
+            ASSERT(md_document);
+            html = md_document->render_to_html();
+        }
 
-        String html = md_document->render_to_html();
         auto html_document = Web::parse_html_document(html);
         page_view.set_document(html_document);
 

Libraries/LibMarkdown/Block.h

@@ -37,7 +37,6 @@ public:
 
     virtual String render_to_html() const = 0;
     virtual String render_for_terminal() const = 0;
-    virtual bool parse(Vector<StringView>::ConstIterator& lines) = 0;
 };
 
 }

Libraries/LibMarkdown/CodeBlock.cpp

@@ -105,16 +105,16 @@ String CodeBlock::render_for_terminal() const
     return builder.build();
 }
 
-bool CodeBlock::parse(Vector<StringView>::ConstIterator& lines)
+OwnPtr<CodeBlock> CodeBlock::parse(Vector<StringView>::ConstIterator& lines)
 {
     if (lines.is_end())
-        return false;
+        return nullptr;
 
     constexpr auto tick_tick_tick = "```";
 
     StringView line = *lines;
     if (!line.starts_with(tick_tick_tick))
-        return false;
+        return nullptr;
 
     // Our Markdown extension: we allow
     // specifying a style and a language
@@ -128,8 +128,9 @@ bool CodeBlock::parse(Vector<StringView>::ConstIterator& lines)
     // and if possible syntax-highlighted
     // as appropriate for a shell script.
     StringView style_spec = line.substring_view(3, line.length() - 3);
-    bool success = m_style_spec.parse(style_spec);
+    auto spec = Text::parse(style_spec);
-    ASSERT(success);
+    if (!spec.has_value())
+        return nullptr;
 
     ++lines;
 
@@ -149,8 +150,7 @@ bool CodeBlock::parse(Vector<StringView>::ConstIterator& lines)
         first = false;
     }
 
-    m_code = builder.build();
+    return make<CodeBlock>(move(spec.value()), builder.build());
-    return true;
 }
 
 }

Libraries/LibMarkdown/CodeBlock.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <AK/OwnPtr.h>
 #include <LibMarkdown/Block.h>
 #include <LibMarkdown/Text.h>
 
@@ -33,11 +34,16 @@ namespace Markdown {
 
 class CodeBlock final : public Block {
 public:
-    virtual ~CodeBlock() override {}
+    CodeBlock(Text&& style_spec, const String& code)
+        : m_code(move(code))
+        , m_style_spec(move(style_spec))
+    {
+    }
+    virtual ~CodeBlock() override { }
 
     virtual String render_to_html() const override;
     virtual String render_for_terminal() const override;
-    virtual bool parse(Vector<StringView>::ConstIterator& lines) override;
+    static OwnPtr<CodeBlock> parse(Vector<StringView>::ConstIterator& lines);
 
 private:
     String style_language() const;

Libraries/LibMarkdown/Document.cpp

@@ -67,19 +67,18 @@ String Document::render_for_terminal() const
 template<typename BlockType>
 static bool helper(Vector<StringView>::ConstIterator& lines, NonnullOwnPtrVector<Block>& blocks)
 {
-    NonnullOwnPtr<BlockType> block = make<BlockType>();
+    OwnPtr<BlockType> block = BlockType::parse(lines);
-    bool success = block->parse(lines);
+    if (!block)
-    if (!success)
         return false;
-    blocks.append(move(block));
+    blocks.append(block.release_nonnull());
     return true;
 }
 
-RefPtr<Document> Document::parse(const StringView& str)
+OwnPtr<Document> Document::parse(const StringView& str)
 {
     const Vector<StringView> lines_vec = str.lines();
     auto lines = lines_vec.begin();
-    auto document = adopt(*new Document);
+    auto document = make<Document>();
     auto& blocks = document->m_blocks;
 
     while (true) {

Libraries/LibMarkdown/Document.h

@@ -32,12 +32,12 @@
 
 namespace Markdown {
 
-class Document final : public RefCounted<Document> {
+class Document final {
 public:
     String render_to_html() const;
     String render_for_terminal() const;
 
-    static RefPtr<Document> parse(const StringView&);
+    static OwnPtr<Document> parse(const StringView&);
 
 private:
     NonnullOwnPtrVector<Block> m_blocks;

Libraries/LibMarkdown/Heading.cpp

@@ -59,26 +59,30 @@ String Heading::render_for_terminal() const
     return builder.build();
 }
 
-bool Heading::parse(Vector<StringView>::ConstIterator& lines)
+OwnPtr<Heading> Heading::parse(Vector<StringView>::ConstIterator& lines)
 {
     if (lines.is_end())
-        return false;
+        return nullptr;
 
     const StringView& line = *lines;
+    size_t level;
 
-    for (m_level = 0; m_level < (int)line.length(); m_level++)
+    for (level = 0; level < line.length(); level++)
-        if (line[(size_t)m_level] != '#')
+        if (line[level] != '#')
             break;
 
-    if (m_level >= (int)line.length() || line[(size_t)m_level] != ' ')
+    if (level >= line.length() || line[level] != ' ')
-        return false;
+        return nullptr;
 
-    StringView title_view = line.substring_view((size_t)m_level + 1, line.length() - (size_t)m_level - 1);
+    StringView title_view = line.substring_view(level + 1, line.length() - level - 1);
-    bool success = m_text.parse(title_view);
+    auto text = Text::parse(title_view);
-    ASSERT(success);
+    if (!text.has_value())
+        return nullptr;
+
+    auto heading = make<Heading>(move(text.value()), level);
 
     ++lines;
-    return true;
+    return heading;
 }
 
 }

Libraries/LibMarkdown/Heading.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <AK/OwnPtr.h>
 #include <AK/StringView.h>
 #include <AK/Vector.h>
 #include <LibMarkdown/Block.h>
@@ -35,15 +36,20 @@ namespace Markdown {
 
 class Heading final : public Block {
 public:
-    virtual ~Heading() override {}
+    Heading(Text&& text, size_t level)
+        : m_text(move(text))
+        , m_level(level)
+    {
+    }
+    virtual ~Heading() override { }
 
     virtual String render_to_html() const override;
     virtual String render_for_terminal() const override;
-    virtual bool parse(Vector<StringView>::ConstIterator& lines) override;
+    static OwnPtr<Heading> parse(Vector<StringView>::ConstIterator& lines);
 
 private:
     Text m_text;
-    int m_level { -1 };
+    size_t m_level { 0 };
 };
 
 }

Libraries/LibMarkdown/List.cpp

@@ -66,21 +66,26 @@ String List::render_for_terminal() const
     return builder.build();
 }
 
-bool List::parse(Vector<StringView>::ConstIterator& lines)
+OwnPtr<List> List::parse(Vector<StringView>::ConstIterator& lines)
 {
+    Vector<Text> items;
+    bool is_ordered = false;
+
     bool first = true;
     size_t offset = 0;
     StringBuilder item_builder;
     auto flush_item_if_needed = [&] {
         if (first)
-            return;
+            return true;
 
-        Text text;
+        auto text = Text::parse(item_builder.string_view());
-        bool success = text.parse(item_builder.string_view());
+        if (!text.has_value())
-        ASSERT(success);
+            return false;
-        m_items.append(move(text));
+
+        items.append(move(text.value()));
 
         item_builder.clear();
+        return true;
     };
 
     while (true) {
@@ -115,21 +120,22 @@ bool List::parse(Vector<StringView>::ConstIterator& lines)
 
         if (appears_unordered || appears_ordered) {
             if (first)
-                m_is_ordered = appears_ordered;
+                is_ordered = appears_ordered;
-            else if (m_is_ordered != appears_ordered)
+            else if (is_ordered != appears_ordered)
-                return false;
+                return nullptr;
 
-            flush_item_if_needed();
+            if (!flush_item_if_needed())
+                return nullptr;
 
             while (offset + 1 < line.length() && line[offset + 1] == ' ')
                 offset++;
 
         } else {
             if (first)
-                return false;
+                return nullptr;
             for (size_t i = 0; i < offset; i++) {
                 if (line[i] != ' ')
-                    return false;
+                    return nullptr;
             }
         }
 
@@ -140,8 +146,9 @@ bool List::parse(Vector<StringView>::ConstIterator& lines)
         ++lines;
     }
 
-    flush_item_if_needed();
+    if (!flush_item_if_needed() || first)
-    return !first;
+        return nullptr;
+    return make<List>(move(items), is_ordered);
 }
 
 }

Libraries/LibMarkdown/List.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <AK/OwnPtr.h>
 #include <AK/Vector.h>
 #include <LibMarkdown/Block.h>
 #include <LibMarkdown/Text.h>
@@ -34,11 +35,17 @@ namespace Markdown {
 
 class List final : public Block {
 public:
+    List(Vector<Text>&& text, bool is_ordered)
+        : m_items(move(text))
+        , m_is_ordered(is_ordered)
+    {
+    }
     virtual ~List() override {}
 
     virtual String render_to_html() const override;
     virtual String render_for_terminal() const override;
-    virtual bool parse(Vector<StringView>::ConstIterator& lines) override;
+
+    static OwnPtr<List> parse(Vector<StringView>::ConstIterator& lines);
 
 private:
     // TODO: List items should be considered blocks of their own kind.

Libraries/LibMarkdown/Paragraph.cpp

@@ -46,10 +46,10 @@ String Paragraph::render_for_terminal() const
     return builder.build();
 }
 
-bool Paragraph::parse(Vector<StringView>::ConstIterator& lines)
+OwnPtr<Paragraph> Paragraph::parse(Vector<StringView>::ConstIterator& lines)
 {
     if (lines.is_end())
-        return false;
+        return nullptr;
 
     bool first = true;
     StringBuilder builder;
@@ -86,11 +86,13 @@ bool Paragraph::parse(Vector<StringView>::ConstIterator& lines)
     }
 
     if (first)
-        return false;
+        return nullptr;
 
-    bool success = m_text.parse(builder.build());
+    auto text = Text::parse(builder.build());
-    ASSERT(success);
+    if (!text.has_value())
-    return true;
+        return nullptr;
+
+    return make<Paragraph>(move(text.value()));
 }
 
 }

Libraries/LibMarkdown/Paragraph.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <AK/OwnPtr.h>
 #include <LibMarkdown/Block.h>
 #include <LibMarkdown/Text.h>
 
@@ -33,11 +34,12 @@ namespace Markdown {
 
 class Paragraph final : public Block {
 public:
+    explicit Paragraph(Text&& text) : m_text(move(text)) {}
     virtual ~Paragraph() override {}
 
     virtual String render_to_html() const override;
     virtual String render_for_terminal() const override;
-    virtual bool parse(Vector<StringView>::ConstIterator& lines) override;
+    static OwnPtr<Paragraph> parse(Vector<StringView>::ConstIterator& lines);
 
 private:
     Text m_text;

Libraries/LibMarkdown/Text.cpp

@@ -181,12 +181,13 @@ String Text::render_for_terminal() const
     return builder.build();
 }
 
-bool Text::parse(const StringView& str)
+Optional<Text> Text::parse(const StringView& str)
 {
     Style current_style;
     size_t current_span_start = 0;
     int first_span_in_the_current_link = -1;
     bool current_link_is_actually_img = false;
+    Vector<Span> spans;
 
     auto append_span_if_needed = [&](size_t offset) {
         ASSERT(current_span_start <= offset);
@@ -195,7 +196,7 @@ bool Text::parse(const StringView& str)
                 unescape(str.substring_view(current_span_start, offset - current_span_start)),
                 current_style
             };
-            m_spans.append(move(span));
+            spans.append(move(span));
             current_span_start = offset;
         }
     };
@@ -239,7 +240,7 @@ bool Text::parse(const StringView& str)
         case '[':
             if (first_span_in_the_current_link != -1)
                 dbg() << "Dropping the outer link";
-            first_span_in_the_current_link = m_spans.size();
+            first_span_in_the_current_link = spans.size();
             break;
         case ']': {
             if (first_span_in_the_current_link == -1) {
@@ -262,11 +263,11 @@ bool Text::parse(const StringView& str)
                 offset--;
 
             const StringView href = str.substring_view(start_of_href, offset - start_of_href);
-            for (size_t i = first_span_in_the_current_link; i < m_spans.size(); i++) {
+            for (size_t i = first_span_in_the_current_link; i < spans.size(); i++) {
                 if (current_link_is_actually_img)
-                    m_spans[i].style.img = href;
+                    spans[i].style.img = href;
                 else
-                    m_spans[i].style.href = href;
+                    spans[i].style.href = href;
             }
             break;
         }
@@ -282,7 +283,7 @@ bool Text::parse(const StringView& str)
 
     append_span_if_needed(str.length());
 
-    return true;
+    return Text(move(spans));
 }
 
 }

Libraries/LibMarkdown/Text.h

@@ -26,12 +26,14 @@
 
 #pragma once
 
+#include <AK/Noncopyable.h>
 #include <AK/String.h>
 #include <AK/Vector.h>
 
 namespace Markdown {
 
 class Text final {
+    AK_MAKE_NONCOPYABLE(Text);
 public:
     struct Style {
         bool emph { false };
@@ -46,14 +48,21 @@ public:
         Style style;
     };
 
+    Text(Text&& text) = default;
+
     const Vector<Span>& spans() const { return m_spans; }
 
     String render_to_html() const;
     String render_for_terminal() const;
 
-    bool parse(const StringView&);
+    static Optional<Text> parse(const StringView&);
 
 private:
+    Text(Vector<Span>&& spans)
+        : m_spans(move(spans))
+    {
+    }
+
     Vector<Span> m_spans;
 };
 

Userland/md.cpp

@@ -25,6 +25,7 @@
  */
 
 #include <AK/ByteBuffer.h>
+#include <AK/OwnPtr.h>
 #include <AK/String.h>
 #include <LibCore/File.h>
 #include <LibMarkdown/Document.h>