Help+man+LibManual: Move argument handling to LibManual

This deduplicates argument handling logic from Help and man and makes it
more modular for future use cases. The argument handling works as
before: two arguments specify section and page (in this order), one
argument specifies either a page (the first section that it's found in
is used) or a path to a manpage markdown file.

Userland/Applications/Help/MainWidget.cpp

@@ -192,47 +192,22 @@ MainWidget::MainWidget()
     };
 }
 
-ErrorOr<void> MainWidget::set_start_page(StringView start_page, u32 section)
+ErrorOr<void> MainWidget::set_start_page(Vector<StringView, 2> query_parameters)
 {
-    bool set_start_page = false;
-    if (!start_page.is_null()) {
-        if (section != 0 && section < Manual::number_of_sections) {
-            // > Help [section] [name]
-            String const path = TRY(TRY(try_make_ref_counted<Manual::PageNode>(Manual::sections[section - 1], TRY(String::from_utf8(start_page))))->path());
-            m_history.push(path);
-            open_page(path);
-            set_start_page = true;
-        } else if (URL url = URL::create_with_url_or_path(start_page); url.is_valid() && url.path().ends_with(".md"sv)) {
-            // > Help [/path/to/documentation/file.md]
-            m_history.push(url.path());
-            open_page(TRY(String::from_deprecated_string(url.path())));
-            set_start_page = true;
-        } else {
-            // > Help [query]
-
-            // First, see if we can find the page by name
-            for (auto const& section : Manual::sections) {
-                String const path = TRY(TRY(try_make_ref_counted<Manual::PageNode>(section, TRY(String::from_utf8(start_page))))->path());
-                if (Core::File::exists(path)) {
-                    m_history.push(path);
-                    open_page(path);
-                    set_start_page = true;
-                    break;
-                }
-            }
-
-            // No match, so treat the input as a search query
-            if (!set_start_page) {
-                m_tab_widget->set_active_widget(m_search_container);
-                m_search_box->set_focus(true);
-                m_search_box->set_text(start_page);
-                m_search_box->select_all();
-                m_filter_model->set_filter_term(m_search_box->text());
-            }
-        }
-    }
-    if (!set_start_page)
+    auto result = Manual::Node::try_create_from_query(query_parameters);
+    if (result.is_error()) {
+        // No match, so treat the input as a search query
+        m_tab_widget->set_active_widget(m_search_container);
+        m_search_box->set_focus(true);
+        m_search_box->set_text(query_parameters.first_matching([](auto&) { return true; }).value_or(""sv));
+        m_search_box->select_all();
+        m_filter_model->set_filter_term(m_search_box->text());
         m_go_home_action->activate();
+    } else {
+        auto const page = TRY(result.value()->path());
+        m_history.push(page);
+        open_page(page);
+    }
     return {};
 }
 

Userland/Applications/Help/MainWidget.h

@@ -20,7 +20,7 @@ public:
     virtual ~MainWidget() override = default;
 
     ErrorOr<void> initialize_fallibles(GUI::Window&);
-    ErrorOr<void> set_start_page(StringView page, u32 section);
+    ErrorOr<void> set_start_page(Vector<StringView, 2> query_parameters);
 
 private:
     MainWidget();

Userland/Applications/Help/main.cpp

@@ -2,6 +2,7 @@
  * Copyright (c) 2019-2020, Sergey Bugaev <bugaevc@serenityos.org>
  * Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
  * Copyright (c) 2021, Sam Atkins <atkinssj@serenityos.org>
+ * Copyright (c) 2022, kleines Filmröllchen <filmroellchen@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -17,15 +18,6 @@
 
 using namespace Help;
 
-static DeprecatedString parse_input(StringView input)
-{
-    AK::URL url = URL::create_with_url_or_path(input);
-    if (url.is_valid())
-        return url.basename();
-
-    return input;
-}
-
 ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
     TRY(Core::System::pledge("stdio recvfd sendfd rpath unix"));
@@ -40,44 +32,27 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     TRY(Core::System::unveil("/tmp/session/%sid/portal/webcontent", "rw"));
     TRY(Core::System::unveil(nullptr, nullptr));
 
-    DeprecatedString start_page;
-    u32 section = 0;
+    DeprecatedString first_query_parameter;
+    DeprecatedString second_query_parameter;
 
     Core::ArgsParser args_parser;
-    // FIXME: These custom Args are a hack. What we want to do is have an optional int arg, then an optional string.
-    // However, when only a string is provided, it gets forwarded to the int argument since that is first, and
-    // parsing fails. This hack instead forwards it to the start_page in that case.
-    args_parser.add_positional_argument(Core::ArgsParser::Arg {
-        .help_string = "Section of the man page",
-        .name = "section",
-        .min_values = 0,
-        .max_values = 1,
-        .accept_value = [&](char const* input_ptr) {
-            StringView input { input_ptr, strlen(input_ptr) };
-            // If it's a number, use it as the section
-            if (auto number = input.to_int(); number.has_value()) {
-                section = number.value();
-                return true;
-            }
-
-            // Otherwise, use it as the start_page
-            start_page = parse_input(input);
-            return true;
-        } });
-    args_parser.add_positional_argument(Core::ArgsParser::Arg {
-        .help_string = "Help page to open. Either an absolute path to the markdown file, or a search query",
-        .name = "page",
-        .min_values = 0,
-        .max_values = 1,
-        .accept_value = [&](char const* input_ptr) {
-            StringView input { input_ptr, strlen(input_ptr) };
-            // If start_page was already set by our section arg, then it can't be set again
-            if (!start_page.is_empty())
-                return false;
-            start_page = parse_input(input);
-            return true;
-        } });
+    // The actual "page query" parsing happens when we set the main widget's start page.
+    args_parser.add_positional_argument(
+        first_query_parameter,
+        "Section of the man page",
+        "section",
+        Core::ArgsParser::Required::No);
+    args_parser.add_positional_argument(
+        second_query_parameter,
+        "Help page to open. Either an absolute path to the markdown file, or a search query",
+        "page",
+        Core::ArgsParser::Required::No);
     args_parser.parse(arguments);
+    Vector<StringView, 2> query_parameters;
+    if (!first_query_parameter.is_empty())
+        query_parameters.append(first_query_parameter);
+    if (!second_query_parameter.is_empty())
+        query_parameters.append(second_query_parameter);
 
     auto app_icon = GUI::Icon::default_icon("app-help"sv);
 
@@ -88,7 +63,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     auto main_widget = TRY(window->try_set_main_widget<MainWidget>());
     TRY(main_widget->initialize_fallibles(window));
-    TRY(main_widget->set_start_page(start_page, section));
+    TRY(main_widget->set_start_page(query_parameters));
 
     window->show();
 

Userland/Libraries/LibManual/CMakeLists.txt

@@ -1,4 +1,5 @@
 set(SOURCES
+    Node.cpp
     PageNode.cpp
     Path.cpp
     SectionNode.cpp

Userland/Libraries/LibManual/Node.cpp

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2022, kleines Filmröllchen <filmroellchen@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include "Node.h"
+#include "PageNode.h"
+#include "SectionNode.h"
+#include <AK/Assertions.h>
+#include <AK/LexicalPath.h>
+#include <AK/Optional.h>
+#include <AK/StringView.h>
+#include <LibCore/File.h>
+#include <LibCore/Stream.h>
+#include <LibManual/Path.h>
+
+namespace Manual {
+
+ErrorOr<NonnullRefPtr<PageNode>> Node::try_create_from_query(Vector<StringView, 2> const& query_parameters)
+{
+    if (query_parameters.size() > 2)
+        return Error::from_string_literal("Queries longer than 2 strings are not supported yet");
+
+    auto query_parameter_iterator = query_parameters.begin();
+
+    if (query_parameter_iterator.is_end())
+        // BUG! No query was given.
+        VERIFY_NOT_REACHED();
+
+    auto first_query_parameter = *query_parameter_iterator;
+    ++query_parameter_iterator;
+    if (query_parameter_iterator.is_end()) {
+        // [/path/to/docs.md]
+        auto path_from_query = LexicalPath { first_query_parameter };
+        if (path_from_query.is_absolute()
+            && path_from_query.is_child_of(manual_base_path)
+            && path_from_query.extension() == "md"sv) {
+            auto section_directory = path_from_query.parent();
+            auto man_string_location = section_directory.basename().find("man"sv);
+            if (!man_string_location.has_value())
+                return Error::from_string_literal("Page is inside invalid section");
+            auto section_name = section_directory.basename().substring_view(man_string_location.value() + 3);
+            auto section = TRY(SectionNode::try_create_from_number(section_name));
+            return try_make_ref_counted<PageNode>(section, TRY(String::from_utf8(path_from_query.title())));
+        }
+
+        // [page] (in any section)
+        Optional<NonnullRefPtr<PageNode>> maybe_page;
+        for (auto const& section : sections) {
+            auto const page = TRY(try_make_ref_counted<PageNode>(section, TRY(String::from_utf8(first_query_parameter))));
+            if (Core::File::exists(TRY(page->path()))) {
+                maybe_page = page;
+                break;
+            }
+        }
+        if (maybe_page.has_value())
+            return maybe_page.release_value();
+        return Error::from_string_literal("Page not found");
+    }
+    // [section] [name]
+    auto second_query_parameter = *query_parameter_iterator;
+    auto section = TRY(SectionNode::try_create_from_number(first_query_parameter));
+    auto const page = TRY(try_make_ref_counted<PageNode>(section, TRY(String::from_utf8(second_query_parameter))));
+    if (Core::File::exists(TRY(page->path())))
+        return page;
+    return Error::from_string_literal("Page doesn't exist in section");
+}
+
+}

Userland/Libraries/LibManual/Node.h

@@ -25,6 +25,13 @@ public:
     virtual ErrorOr<String> name() const = 0;
     virtual bool is_page() const { return false; }
     virtual bool is_open() const { return false; }
+
+    // Backend for the command-line argument format that Help and man accept. Handles:
+    // [/path/to/documentation.md] (no second argument)
+    // [page] (no second argument) - will find first section with that page
+    // [section] [page]
+    // Help can also (externally) handle search queries, which is not possible (yet) in man.
+    static ErrorOr<NonnullRefPtr<PageNode>> try_create_from_query(Vector<StringView, 2> const& query_parameters);
 };
 
 }

Userland/Libraries/LibManual/SectionNode.cpp

@@ -13,6 +13,17 @@
 
 namespace Manual {
 
+ErrorOr<NonnullRefPtr<SectionNode>> SectionNode::try_create_from_number(StringView section)
+{
+    auto maybe_section_number = section.to_uint<u32>();
+    if (!maybe_section_number.has_value())
+        return Error::from_string_literal("Section is not a number");
+    auto section_number = maybe_section_number.release_value();
+    if (section_number > number_of_sections)
+        return Error::from_string_literal("Section number too large");
+    return sections[section_number - 1];
+}
+
 ErrorOr<String> SectionNode::path() const
 {
     return String::formatted("{}/{}{}", manual_base_path, top_level_section_prefix, m_section);

Userland/Libraries/LibManual/SectionNode.h

@@ -37,6 +37,8 @@ public:
     virtual bool is_open() const override { return m_open; }
     void set_open(bool open);
 
+    static ErrorOr<NonnullRefPtr<SectionNode>> try_create_from_number(StringView section_number);
+
 protected:
     String m_section;
     String m_name;

Userland/Utilities/man.cpp

@@ -7,11 +7,13 @@
 #include <AK/Assertions.h>
 #include <AK/ByteBuffer.h>
 #include <AK/DeprecatedString.h>
+#include <AK/Utf8View.h>
 #include <LibCore/ArgsParser.h>
 #include <LibCore/File.h>
 #include <LibCore/Stream.h>
 #include <LibCore/System.h>
 #include <LibMain/Main.h>
+#include <LibManual/Node.h>
 #include <LibManual/PageNode.h>
 #include <LibManual/SectionNode.h>
 #include <LibMarkdown/Document.h>
@@ -56,64 +58,45 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     TRY(Core::System::unveil("/bin", "x"));
     TRY(Core::System::unveil(nullptr, nullptr));
 
-    DeprecatedString section;
-    DeprecatedString name;
+    DeprecatedString section_argument;
+    DeprecatedString name_argument;
     DeprecatedString pager;
 
     Core::ArgsParser args_parser;
     args_parser.set_general_help("Read manual pages. Try 'man man' to get started.");
-    args_parser.add_positional_argument(section, "Section of the man page", "section", Core::ArgsParser::Required::No);
-    args_parser.add_positional_argument(name, "Name of the man page", "name");
+    args_parser.add_positional_argument(section_argument, "Section of the man page", "section");
+    args_parser.add_positional_argument(name_argument, "Name of the man page", "name", Core::ArgsParser::Required::No);
     args_parser.add_option(pager, "Pager to pipe the man page to", "pager", 'P', "pager");
     args_parser.parse(arguments);
+    Vector<StringView, 2> query_parameters;
+    if (!section_argument.is_empty())
+        query_parameters.append(section_argument);
+    if (!name_argument.is_empty())
+        query_parameters.append(name_argument);
 
-    Optional<NonnullRefPtr<Manual::PageNode>> page;
-    if (section.is_empty()) {
-        for (auto const& s : Manual::sections) {
-            auto const maybe_page = make_ref_counted<Manual::PageNode>(s, TRY(String::from_utf8(name)));
-            if (Core::File::exists(TRY(maybe_page->path()).to_deprecated_string())) {
-                page = maybe_page;
-                section = s->section_name().to_deprecated_string();
-                break;
-            }
-        }
-    } else {
-        auto number_section = section.to_uint();
-        if (number_section.has_value())
-            page = make_ref_counted<Manual::PageNode>(Manual::sections[number_section.value() - 1], TRY(String::from_utf8(name)));
-        else
-            warnln("Section name '{}' invalid", section);
-    }
-
-    if (!page.has_value()) {
-        warnln("No man page for {}", name);
-        exit(1);
-    } else if (!Core::File::exists(TRY((*page)->path()))) {
-        warnln("No man page for {} in section {}", name, section);
-        exit(1);
-    }
+    auto page = TRY(Manual::Node::try_create_from_query(query_parameters));
+    auto page_name = TRY(page->name());
+    auto const* section = static_cast<Manual::SectionNode const*>(page->parent());
 
     if (pager.is_empty())
-        pager = TRY(String::formatted("less -P 'Manual Page {}({}) line %l?e (END):.'", StringView(name).replace("'"sv, "'\\''"sv, ReplaceMode::FirstOnly), StringView(section).replace("'"sv, "'\\''"sv, ReplaceMode::FirstOnly))).to_deprecated_string();
+        pager = TRY(String::formatted("less -P 'Manual Page {}({}) line %l?e (END):.'",
+                        TRY(page_name.replace("'"sv, "'\\''"sv, ReplaceMode::FirstOnly)),
+                        TRY(section->section_name().replace("'"sv, "'\\''"sv, ReplaceMode::FirstOnly))))
+                    .to_deprecated_string();
     pid_t pager_pid = TRY(pipe_to_pager(pager));
 
-    auto file = TRY(Core::Stream::File::open(TRY((*page)->path()), Core::Stream::OpenMode::Read));
+    auto file = TRY(Core::Stream::File::open(TRY(page->path()), Core::Stream::OpenMode::Read));
 
     TRY(Core::System::pledge("stdio proc"));
 
-    dbgln("Loading man page from {}", (*page)->path());
+    dbgln("Loading man page from {}", TRY(page->path()));
     auto buffer = TRY(file->read_all());
     auto source = DeprecatedString::copy(buffer);
 
-    const DeprecatedString title("SerenityOS manual");
+    auto const title = TRY(String::from_utf8("SerenityOS manual"sv));
 
-    int spaces = view_width / 2 - DeprecatedString(name).length() - DeprecatedString(section).length() - title.length() / 2 - 4;
-    if (spaces < 0)
-        spaces = 0;
-    out("{}({})", name, section);
-    while (spaces--)
-        out(" ");
-    outln(title);
+    int spaces = max(view_width / 2 - page_name.code_points().length() - section->section_name().code_points().length() - title.code_points().length() / 2 - 4, 0);
+    outln("{}({}){}{}", page_name, section->section_name(), DeprecatedString::repeated(' ', spaces), title);
 
     auto document = Markdown::Document::parse(source);
     VERIFY(document);