From b65258c093a14a8bac082aa3580ae70b78f92283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= Date: Tue, 12 Jul 2022 20:23:28 +0200 Subject: [PATCH] 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 | 53 ++++----------- Userland/Applications/Help/MainWidget.h | 2 +- Userland/Applications/Help/main.cpp | 65 ++++++------------ Userland/Libraries/LibManual/CMakeLists.txt | 1 + Userland/Libraries/LibManual/Node.cpp | 70 ++++++++++++++++++++ Userland/Libraries/LibManual/Node.h | 7 ++ Userland/Libraries/LibManual/SectionNode.cpp | 11 +++ Userland/Libraries/LibManual/SectionNode.h | 2 + Userland/Utilities/man.cpp | 63 +++++++----------- 9 files changed, 149 insertions(+), 125 deletions(-) create mode 100644 Userland/Libraries/LibManual/Node.cpp diff --git a/Userland/Applications/Help/MainWidget.cpp b/Userland/Applications/Help/MainWidget.cpp index 50a6eece2d..0ba220d696 100644 --- a/Userland/Applications/Help/MainWidget.cpp +++ b/Userland/Applications/Help/MainWidget.cpp @@ -192,47 +192,22 @@ MainWidget::MainWidget() }; } -ErrorOr MainWidget::set_start_page(StringView start_page, u32 section) +ErrorOr MainWidget::set_start_page(Vector 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::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(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 {}; } diff --git a/Userland/Applications/Help/MainWidget.h b/Userland/Applications/Help/MainWidget.h index 8f1012a84b..4b30e64193 100644 --- a/Userland/Applications/Help/MainWidget.h +++ b/Userland/Applications/Help/MainWidget.h @@ -20,7 +20,7 @@ public: virtual ~MainWidget() override = default; ErrorOr initialize_fallibles(GUI::Window&); - ErrorOr set_start_page(StringView page, u32 section); + ErrorOr set_start_page(Vector query_parameters); private: MainWidget(); diff --git a/Userland/Applications/Help/main.cpp b/Userland/Applications/Help/main.cpp index 3ae157664b..ec4c867387 100644 --- a/Userland/Applications/Help/main.cpp +++ b/Userland/Applications/Help/main.cpp @@ -2,6 +2,7 @@ * Copyright (c) 2019-2020, Sergey Bugaev * Copyright (c) 2021, Andreas Kling * Copyright (c) 2021, Sam Atkins + * Copyright (c) 2022, kleines Filmröllchen * * 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 serenity_main(Main::Arguments arguments) { TRY(Core::System::pledge("stdio recvfd sendfd rpath unix")); @@ -40,44 +32,27 @@ ErrorOr 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 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 serenity_main(Main::Arguments arguments) auto main_widget = TRY(window->try_set_main_widget()); 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(); diff --git a/Userland/Libraries/LibManual/CMakeLists.txt b/Userland/Libraries/LibManual/CMakeLists.txt index 0bc2104a03..56258cf52b 100644 --- a/Userland/Libraries/LibManual/CMakeLists.txt +++ b/Userland/Libraries/LibManual/CMakeLists.txt @@ -1,4 +1,5 @@ set(SOURCES + Node.cpp PageNode.cpp Path.cpp SectionNode.cpp diff --git a/Userland/Libraries/LibManual/Node.cpp b/Userland/Libraries/LibManual/Node.cpp new file mode 100644 index 0000000000..e8720f88ab --- /dev/null +++ b/Userland/Libraries/LibManual/Node.cpp @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2022, kleines Filmröllchen + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "Node.h" +#include "PageNode.h" +#include "SectionNode.h" +#include +#include +#include +#include +#include +#include +#include + +namespace Manual { + +ErrorOr> Node::try_create_from_query(Vector 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(section, TRY(String::from_utf8(path_from_query.title()))); + } + + // [page] (in any section) + Optional> maybe_page; + for (auto const& section : sections) { + auto const page = TRY(try_make_ref_counted(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(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"); +} + +} diff --git a/Userland/Libraries/LibManual/Node.h b/Userland/Libraries/LibManual/Node.h index 9b56aaab94..46347bd8e4 100644 --- a/Userland/Libraries/LibManual/Node.h +++ b/Userland/Libraries/LibManual/Node.h @@ -25,6 +25,13 @@ public: virtual ErrorOr 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> try_create_from_query(Vector const& query_parameters); }; } diff --git a/Userland/Libraries/LibManual/SectionNode.cpp b/Userland/Libraries/LibManual/SectionNode.cpp index deae4a482d..1d18e77613 100644 --- a/Userland/Libraries/LibManual/SectionNode.cpp +++ b/Userland/Libraries/LibManual/SectionNode.cpp @@ -13,6 +13,17 @@ namespace Manual { +ErrorOr> SectionNode::try_create_from_number(StringView section) +{ + auto maybe_section_number = section.to_uint(); + 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 SectionNode::path() const { return String::formatted("{}/{}{}", manual_base_path, top_level_section_prefix, m_section); diff --git a/Userland/Libraries/LibManual/SectionNode.h b/Userland/Libraries/LibManual/SectionNode.h index 04bd7a568d..414e03b17c 100644 --- a/Userland/Libraries/LibManual/SectionNode.h +++ b/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> try_create_from_number(StringView section_number); + protected: String m_section; String m_name; diff --git a/Userland/Utilities/man.cpp b/Userland/Utilities/man.cpp index 8e53347adb..7d0117496a 100644 --- a/Userland/Utilities/man.cpp +++ b/Userland/Utilities/man.cpp @@ -7,11 +7,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -56,64 +58,45 @@ ErrorOr 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 query_parameters; + if (!section_argument.is_empty()) + query_parameters.append(section_argument); + if (!name_argument.is_empty()) + query_parameters.append(name_argument); - Optional> page; - if (section.is_empty()) { - for (auto const& s : Manual::sections) { - auto const maybe_page = make_ref_counted(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::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(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);