From 9cf9531accaf93abdcb3e43321e720d42a8e7a73 Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Sat, 25 Jul 2026 18:58:54 +0300 Subject: [PATCH] mu: improve command-line handling - Remove some dead code - Make error messages a bit more user-friendly. - Small cleanups - Add unit tests and actually run them --- mu/mu-cmd-count.cc | 647 ------------------------------------------- mu/mu-options.cc | 480 ++++++++++++++++++++++---------- mu/mu-options.hh | 14 +- mu/mu.cc | 2 - mu/tests/meson.build | 9 + 5 files changed, 342 insertions(+), 810 deletions(-) delete mode 100644 mu/mu-cmd-count.cc diff --git a/mu/mu-cmd-count.cc b/mu/mu-cmd-count.cc deleted file mode 100644 index 8a8165f3..00000000 --- a/mu/mu-cmd-count.cc +++ /dev/null @@ -1,647 +0,0 @@ - /* - ** Copyright (C) 2024 Dirk-Jan C. Binnema - ** - ** This program is free software; you can redistribute it and/or modify it - ** under the terms of the GNU General Public License as published by the - ** Free Software Foundation; either version 3, or (at your option) any - ** later version. - ** - ** This program is distributed in the hope that it will be useful, - ** but WITHOUT ANY WARRANTY; without even the implied warranty of - ** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - ** GNU General Public License for more details. - ** - ** You should have received a copy of the GNU General Public License - ** along with this program; if not, write to the Free Software Foundation, - ** Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - ** - */ - -#include "config.h" - -#include - -#include -#include -#include -#include -#include -#include -#include - -#include "message/mu-message.hh" -#include "mu-maildir.hh" -#include "mu-query-match-deciders.hh" -#include "mu-query.hh" -#include "mu-query-macros.hh" -#include "mu-query-parser.hh" -#include "message/mu-message.hh" - -#include "utils/mu-option.hh" - -#include "mu-cmd.hh" -#include "utils/mu-utils.hh" - -using namespace Mu; - -static Result -count_query(const Store& store, const Options& opts) -{ - if (opts.count.query.empty()) - return Err(Error::Code::InvalidArgument, - "missing query"); - - auto&& query{join(opts.count.query, " ")}; - - return Ok(store.count_query(query)); -} - -static Result -get_query(const Store& store, const Options& opts) -{ - if (opts.find.bookmark.empty() && opts.find.query.empty()) - return Err(Error::Code::InvalidArgument, - "neither bookmark nor query"); - - std::string bookmark; - if (!opts.find.bookmark.empty()) { - const auto res = resolve_bookmark(store, opts); - if (!res) - return Err(std::move(res.error())); - bookmark = res.value() + " "; - } - - auto&& query{join(opts.find.query, " ")}; - return Ok(bookmark + query); -} - -static Result -prepare_links(const Options& opts) -{ - /* note, mu_maildir_mkdir simply ignores whatever part of the - * mail dir already exists */ - if (auto&& res = maildir_mkdir(opts.find.linksdir, 0700, true); !res) - return Err(std::move(res.error())); - - if (!opts.find.clearlinks) - return Ok(); - - if (auto&& res = maildir_clear_links(opts.find.linksdir); !res) - return Err(std::move(res.error())); - - return Ok(); -} - -static Result -output_link(const Option& msg, const OutputInfo& info, const Options& opts) -{ - if (info.header) - return prepare_links(opts); - else if (info.footer) - return Ok(); - - /* during test, do not create "unique names" (i.e., names with path - * hashes), so we get a predictable result */ - const auto unique_names{!g_getenv("MU_TEST")&&!g_test_initialized()}; - - if (auto&& res = maildir_link(msg->path(), opts.find.linksdir, unique_names); !res) - return Err(std::move(res.error())); - - return Ok(); -} - -static void -ansi_color_maybe(Field::Id field_id, bool color) -{ - const char* ansi; - - if (!color) - return; /* nothing to do */ - - switch (field_id) { - case Field::Id::From: ansi = MU_COLOR_CYAN; break; - - case Field::Id::To: - case Field::Id::Cc: - case Field::Id::Bcc: ansi = MU_COLOR_BLUE; break; - case Field::Id::Subject: ansi = MU_COLOR_GREEN; break; - case Field::Id::Date: ansi = MU_COLOR_MAGENTA; break; - - default: - if (field_from_id(field_id).type != Field::Type::String) - ansi = MU_COLOR_YELLOW; - else - ansi = MU_COLOR_RED; - } - - fputs(ansi, stdout); -} - -static void -ansi_reset_maybe(Field::Id field_id, bool color) -{ - if (!color) - return; /* nothing to do */ - - fputs(MU_COLOR_DEFAULT, stdout); -} - -static std::string -display_field(const Message& msg, Field::Id field_id) -{ - switch (field_from_id(field_id).type) { - case Field::Type::String: - return msg.document().string_value(field_id); - case Field::Type::Integer: - if (field_id == Field::Id::Priority) { - return to_string(msg.priority()); - } else if (field_id == Field::Id::Flags) { - return to_string(msg.flags()); - } else /* as string */ - return msg.document().string_value(field_id); - case Field::Type::TimeT: - return mu_format("{:%c}", - mu_time(msg.document().integer_value(field_id))); - case Field::Type::ByteSize: - return to_string(msg.document().integer_value(field_id)); - case Field::Type::StringList: - return join(msg.document().string_vec_value(field_id), ','); - case Field::Type::ContactList: - return to_string(msg.document().contacts_value(field_id)); - default: - g_return_val_if_reached(""); - return ""; - } -} - -static void -print_summary(const Message& msg, const Options& opts) -{ - const auto body{msg.body_text()}; - if (!body) - return; - - const auto summ{summarize(body->c_str(), opts.find.summary_len.value_or(0))}; - - mu_print("Summary: "); - fputs_encoded(summ, stdout); - mu_println(""); -} - -static void -thread_indent(const QueryMatch& info, const Options& opts) -{ - const auto is_root{any_of(info.flags & QueryMatch::Flags::Root)}; - const auto first_child{any_of(info.flags & QueryMatch::Flags::First)}; - const auto last_child{any_of(info.flags & QueryMatch::Flags::Last)}; - const auto empty_parent{any_of(info.flags & QueryMatch::Flags::Orphan)}; - const auto is_dup{any_of(info.flags & QueryMatch::Flags::Duplicate)}; - // const auto is_related{any_of(info.flags & QueryMatch::Flags::Related)}; - - /* indent */ - if (opts.debug) { - ::fputs(info.thread_path.c_str(), stdout); - ::fputs(" ", stdout); - } else - for (auto i = info.thread_level; i > 1; --i) - ::fputs(" ", stdout); - - if (!is_root) { - if (first_child) - ::fputs("\\", stdout); - else if (last_child) - ::fputs("/", stdout); - else - ::fputs(" ", stdout); - ::fputs(empty_parent ? "*> " : is_dup ? "=> " - : "-> ", - stdout); - } -} - -static void -output_plain_fields(const Message& msg, const std::string& fields, - bool color, bool threads) -{ - size_t nonempty{}; - - for (auto&& k: fields) { - const auto field_opt{field_from_shortcut(k)}; - if (!field_opt || (!field_opt->is_value() && !field_opt->is_contact())) - nonempty += printf("%c", k); - - else { - ansi_color_maybe(field_opt->id, color); - nonempty += fputs_encoded( - display_field(msg, field_opt->id), stdout); - ansi_reset_maybe(field_opt->id, color); - } - } - - if (nonempty) - fputs("\n", stdout); -} - -static Result -output_plain(const Option& msg, const OutputInfo& info, - const Options& opts) -{ - if (!msg) - return Ok(); - - /* we reuse the color (whatever that may be) - * for message-priority for threads, too */ - ansi_color_maybe(Field::Id::Priority, !opts.nocolor); - if (opts.find.threads && info.match_info) - thread_indent(*info.match_info, opts); - - output_plain_fields(*msg, opts.find.fields, !opts.nocolor, opts.find.threads); - - if (opts.view.summary_len) - print_summary(*msg, opts); - - return Ok(); -} - -static Result -output_sexp(const Option& msg, const OutputInfo& info, const Options& opts) -{ - if (msg) { - if (const auto sexp{msg->sexp()}; !sexp.empty()) - fputs(sexp.to_string().c_str(), stdout); - else - fputs(msg->sexp().to_string().c_str(), stdout); - fputs("\n", stdout); - } - - return Ok(); -} - -static Result -output_json(const Option& msg, const OutputInfo& info, const Options& opts) -{ - if (info.header) { - mu_println("["); - return Ok(); - } - - if (info.footer) { - mu_println("]"); - return Ok(); - } - - if (!msg) - return Ok(); - - mu_println("{}{}", msg->sexp().to_json_string(), info.last ? "" : ","); - - return Ok(); -} - -static void -print_attr_xml(const std::string& elm, const std::string& str) -{ - if (str.empty()) - return; /* empty: don't include */ - - auto&& esc{to_string_opt_gchar(g_markup_escape_text(str.c_str(), -1))}; - mu_println("\t\t<{}>{}", elm, esc.value_or(""), elm); -} - -static Result -output_xml(const Option& msg, const OutputInfo& info, const Options& opts) -{ - if (info.header) { - mu_println(""); - mu_println(""); - return Ok(); - } - - if (info.footer) { - mu_println(""); - return Ok(); - } - - mu_println("\t"); - print_attr_xml("from", to_string(msg->from())); - print_attr_xml("to", to_string(msg->to())); - print_attr_xml("cc", to_string(msg->cc())); - print_attr_xml("subject", msg->subject()); - mu_println("\t\t{}", (unsigned)msg->date()); - mu_println("\t\t{}", (unsigned)msg->size()); - print_attr_xml("msgid", msg->message_id()); - print_attr_xml("path", msg->path()); - print_attr_xml("maildir", msg->maildir()); - mu_println("\t"); - - return Ok(); -} - -static OutputFunc -get_output_func(const Options& opts) -{ - if (!opts.find.exec.empty()) - return exec_cmd; - - switch (opts.find.format) { - case Format::Links: - return output_link; - case Format::Plain: - return output_plain; - case Format::Xml: - return output_xml; - case Format::Sexp: - return output_sexp; - case Format::Json: - return output_json; - default: - throw Error(Error::Code::Internal, - "invalid format {}", - static_cast(opts.find.format)); - } -} - -static Result -output_query_results(const QueryResults& qres, const Options& opts) -{ - GError* err{}; - const auto output_func{get_output_func(opts)}; - if (!output_func) - return Err(Error::Code::Query, &err, "failed to find output function"); - - if (auto&& res = output_func(Nothing, FirstOutput, opts); !res) - return Err(std::move(res.error())); - - size_t n{0}; - for (auto&& item : qres) { - n++; - auto msg{item.message()}; - if (!msg) - continue; - - if (msg->changed() < opts.find.after.value_or(0)) - continue; - - if (auto&& res = output_func(msg, - {item.doc_id(), - false, - false, - n == qres.size(), /* last? */ - item.query_match()}, - opts); !res) - return Err(std::move(res.error())); - } - - if (auto&& res{output_func(Nothing, LastOutput, opts)}; !res) - return Err(std::move(res.error())); - else - return Ok(); -} - -static Result -process_store_query(const Store& store, const std::string& expr, const Options& opts) -{ - auto qres{run_query(store, expr, opts)}; - if (!qres) - return Err(qres.error()); - - if (qres->empty()) - return Err(Error::Code::NoMatches, "no matches for search expression"); - - return output_query_results(*qres, opts); -} - -Result -Mu::mu_cmd_find(const Store& store, const Options& opts) -{ - auto expr{get_query(store, opts)}; - if (!expr) - return Err(expr.error()); - - if (opts.find.analyze) - return analyze_query_expr(store, *expr, opts); - else - return process_store_query(store, *expr, opts); -} - - - -#ifdef BUILD_TESTS -/* - * Tests. - * - */ - -#include "utils/mu-test-utils.hh" - - -/* tests for the command line interface, uses testdir2 */ - -static std::string test_mu_home; - -auto count_nl(const std::string& s)->size_t { - size_t n{}; - for (auto&& c: s) - if (c == '\n') - ++n; - return n; -} - -static size_t -search_func(const std::string& expr, size_t expected) -{ - auto res = run_command({MU_PROGRAM, "find", "--muhome", test_mu_home, expr}); - assert_valid_result(res); - - /* we expect zero lines of error output if there is a match; otherwise - * there should be one line 'No matches found' */ - if (res->exit_code != 0) { - g_assert_cmpuint(res->exit_code, ==, 2); // no match - g_assert_true(res->standard_out.empty()); - g_assert_cmpuint(count_nl(res->standard_err), ==, 1); - return 0; - } - - return count_nl(res->standard_out); -} - -#define search(Q,EXP) do { \ - g_assert_cmpuint(search_func(Q, EXP), ==, EXP); \ -} while(0) - - -static void -test_mu_find_empty_query(void) -{ - search("\"\"", 14); -} - -static void -test_mu_find_01(void) -{ - search("f:john fruit", 1); - search("f:soc@example.com", 1); - search("t:alki@example.com", 1); - search("t:alcibiades", 1); - search("http emacs", 1); - search("f:soc@example.com OR f:john", 2); - search("f:soc@example.com OR f:john OR t:edmond", 3); - search("t:julius", 1); - search("s:dude", 1); - search("t:dantès", 1); -} - -/* index testdir2, and make sure it adds two documents */ -static void -test_mu_find_02(void) -{ - search("bull", 1); - search("g:x", 0); - search("flag:encrypted", 0); - search("flag:attach", 1); - - search("i:3BE9E6535E0D852173@emss35m06.us.lmco.com", 1); -} - -static void -test_mu_find_file(void) -{ - search("file:sittingbull.jpg", 1); - search("file:custer.jpg", 1); - search("file:custer.*", 1); - search("j:sit*", 1); -} - -static void -test_mu_find_mime(void) -{ - search("mime:image/jpeg", 1); - search("mime:text/plain", 14); - search("y:text*", 14); - search("y:image*", 1); - search("mime:message/rfc822", 2); -} - -static void -test_mu_find_text_in_rfc822(void) -{ - search("embed:dancing", 1); - search("e:curious", 1); - search("embed:with", 2); - search("e:karjala", 0); - search("embed:navigation", 1); -} - -static void -test_mu_find_maildir_special(void) -{ - search("\"maildir:/wOm_bàT\"", 3); - search("\"maildir:/wOm*\"", 3); - search("\"maildir:/wOm_*\"", 3); - search("\"maildir:wom_bat\"", 0); - search("\"maildir:/wombat\"", 0); - search("subject:atoms", 1); - search("\"maildir:/wom_bat\" subject:atoms", 1); -} - - -/* some more tests */ - -static void -test_mu_find_wrong_muhome() -{ - auto res = run_command({MU_PROGRAM, "find", "--muhome", - join_paths("/foo", "bar", "nonexistent"), "f:socrates"}); - assert_valid_result(res); - g_assert_cmpuint(res->exit_code,==,1); // general error - g_assert_cmpuint(count_nl(res->standard_err), >, 1); -} - -static void -test_mu_find_links(void) -{ - TempDir temp_dir; - - { - auto res = run_command({MU_PROGRAM, "find", "--muhome", test_mu_home, - "--format", "links", "--linksdir", temp_dir.path(), - "mime:message/rfc822"}); - assert_valid_result(res); - g_assert_cmpuint(res->exit_code,==,0); - g_assert_cmpuint(count_nl(res->standard_out),==,0); - g_assert_cmpuint(count_nl(res->standard_err),==,0); - } - - - /* furthermore, two symlinks should be there */ - const auto f1{mu_format("{}/cur/rfc822.1", temp_dir)}; - const auto f2{mu_format("{}/cur/rfc822.2", temp_dir)}; - - g_assert_cmpuint(determine_dtype(f1.c_str(), true), ==, DT_LNK); - g_assert_cmpuint(determine_dtype(f2.c_str(), true), ==, DT_LNK); - - /* now we try again, we should get a line of error output, - * when we find the first target file already exists */ - { - auto res = run_command({MU_PROGRAM, "find", "--muhome", test_mu_home, - "--format", "links", "--linksdir", temp_dir.path(), - "mime:message/rfc822"}); - assert_valid_result(res); - g_assert_cmpuint(res->exit_code,==,1); - g_assert_cmpuint(count_nl(res->standard_out),==,0); - g_assert_cmpuint(count_nl(res->standard_err),==,1); - } - - /* now we try again with --clearlinks, and the we should be - * back to 0 errors */ - { - auto res = run_command({MU_PROGRAM, "find", "--muhome", test_mu_home, - "--format", "links", "--clearlinks", "--linksdir", temp_dir.path(), - "mime:message/rfc822"}); - assert_valid_result(res); - g_assert_cmpuint(res->exit_code,==,0); - g_assert_cmpuint(count_nl(res->standard_out),==,0); - g_assert_cmpuint(count_nl(res->standard_err),==,0); - } - - g_assert_cmpuint(determine_dtype(f1.c_str(), true), ==, DT_LNK); - g_assert_cmpuint(determine_dtype(f2.c_str(), true), ==, DT_LNK); -} - -/* some more tests */ - -int -main(int argc, char* argv[]) -{ - mu_test_init(&argc, &argv); - - if (!set_en_us_utf8_locale()) - return 0; /* don't error out... */ - - TempDir temp_dir{}; - { - test_mu_home = temp_dir.path(); - - auto res1 = run_command({MU_PROGRAM, "--quiet", "init", - "--muhome", test_mu_home, "--maildir" , MU_TESTMAILDIR2}); - assert_valid_result(res1); - - auto res2 = run_command({MU_PROGRAM, "--quiet", "index", - "--muhome", test_mu_home}); - assert_valid_result(res2); - } - - g_test_add_func("/cmd/find/empty-query", test_mu_find_empty_query); - g_test_add_func("/cmd/find/01", test_mu_find_01); - g_test_add_func("/cmd/find/02", test_mu_find_02); - g_test_add_func("/cmd/find/file", test_mu_find_file); - g_test_add_func("/cmd/find/mime", test_mu_find_mime); - g_test_add_func("/cmd/find/links", test_mu_find_links); - g_test_add_func("/cmd/find/text-in-rfc822", test_mu_find_text_in_rfc822); - g_test_add_func("/cmd/find/wrong-muhome", test_mu_find_wrong_muhome); - g_test_add_func("/cmd/find/maildir-special", test_mu_find_maildir_special); - - return g_test_run(); -} - -#endif /*BUILD_TESTS*/ diff --git a/mu/mu-options.cc b/mu/mu-options.cc index 5d557ae2..23a8e631 100644 --- a/mu/mu-options.cc +++ b/mu/mu-options.cc @@ -20,20 +20,18 @@ /** * @brief Command-line handling * - * Here we implement mu's command-line parsing based on the CLI11 library. At - * the time of writing, that library seems to be the best based on the criteria + * Here we implement mu's command-line parsing based on the CLI11 library. + * + * At the time of writing, that library seems to be the best based on the criteria * that it supports the features we need and is available as a header-only * include. * * CLI11 can do quite a bit, and we're only scratching the surface here, * plan is to slowly improve things. - * - * - we do quite a bit of sanity-checking, but the errors are a rather terse - * - the docs could be improved, e.g., `mu find --help` and --format/--sortfield - * */ #include +#include #include #include #include @@ -59,13 +57,13 @@ using namespace Mu; /* * helpers */ - +namespace { /** - * array of associated pair elements -- like an alist - * but based on std::array and thus can be constexpr + * Element of an array of associated pair elements -- like an alist + * entry; make the array with std::to_array so its size is deduced. */ -template - using AssocPairs = std::array, N>; +template + using AssocPair = std::pair; /** * Get the first value of the pair where the second element is @param s. @@ -104,73 +102,16 @@ to_second(const P& p, typename P::value_type::first_type f) } /** - * Options-specific array-bases type that maps some enum to a pair + * Options-specific pair type that maps some enum to a pair */ -template -using InfoEnum = AssocPairs, N>; - -/** - * Get the name (shortname) for some InfoEnum, based on the enum - * - * @param ie an InfoEnum - * @param e an enum value - * - * @return the name if found, or Nothing - */ -template -static constexpr Option -to_name(const IE& ie, typename IE::value_type::first_type e) { - if (auto&& s{to_second(ie, e)}; s) - return s->first; - else - return Nothing; -} - -/** - * Get the enum value for some InfoEnum, based on the name - * - * @param ie an InfoEnum - * @param name some name (shortname) - * - * @return the name if found, or Nothing - */ -template -static constexpr Option -to_enum(const IE& ie, std::string_view name) { - for(auto&& item: ie) - if (item.second.first == name) - return item.first; - return Nothing; -} - -/** - * List help options for as a string, with the default marked with '(*)' - * - * @param ie infoenum - * @param default_opt default option - * - * @return a help string - */ -template -static std::string -options_help(const IE& ie, typename IE::value_type::first_type default_opt) -{ - std::string s; - for(auto&& item: ie) { - if (!s.empty()) - s += ", "; - s += std::string{item.second.first}; - if (item.first == default_opt) - s += "(*)"; /* default option */ - } - return s; -} +template +using InfoPair = AssocPair>; /** * Get map from string->type */ template -static std::unordered_map +std::unordered_map options_map(const IE& ie) { std::unordered_map map; @@ -180,21 +121,166 @@ options_map(const IE& ie) return map; } +/** + * Get the list of option names (shortnames), in declaration order + */ +template +std::vector +options_names(const IE& ie) +{ + std::vector names; + names.reserve(ie.size()); + for (auto&& item : ie) + names.emplace_back(item.second.first); + + return names; +} + +/** + * Friendly error messages for some options (see add_choice_option, + * add_number_option), used for rewriting CLI11's rather terse errors. + */ +struct FriendlyError { + const CLI::App *sub; /**< subcommand owning the option */ + const CLI::Option *opt; /**< the option */ + std::string errmsg; /**< friendly error message */ +}; +std::vector friendly_errors; + +/** + * Get a friendly error message for some error. + * + * @param err a parse error + * + * @return the error message + */ +std::string +friendly_error(const CLI::ParseError& err) +{ + const std::string what{err.what()}; + for (const auto& info: friendly_errors) { + if (!info.sub->parsed()) + continue; + const auto name{info.opt->get_name()}; + // the CLI11 messages we know how to improve upon + for (const auto pat: { ": 1 required", // no value given + ": requires one of", // bad choice value + ": Value " }) // bad numeric value + if (what.starts_with(name + pat)) + return info.errmsg; + } + return what; +} + +/** + * Add an option to sub that takes its value from a fixed set of choices. + * + * Like CLI11's CheckedTransformer, but with friendlier help and error + * messages, e.g. + * error: --format requires one of { plain, links }; default is plain + * + * @param sub command to add the option to + * @param name option name(s), e.g. "--format,-o" + * @param value target for the parsed value + * @param help help text; the choices and default are appended + * @param type_name name for the value in the help, e.g. "" + * @param choices map from choice-name to value + * @param choice_names the choice names; displayed in alphabetical order + * @param default_name name of the default choice; must occur in @p choices + * + * @return the newly added option + */ +template +CLI::Option* +add_choice_option(CLI::App& sub, const std::string& name, T& value, + const std::string& help, const std::string& type_name, + std::unordered_map choices, + std::vector choice_names, + const std::string& default_name) +{ + std::ranges::sort(choice_names); + const auto choices_str{"{ " + join(choice_names, ", ") + " }"}; + + auto opt = sub.add_option(name, value, + mu_format("{}; one of {}; default is {}", + help, choices_str, default_name)) + ->type_name(type_name) + ->default_val(choices.at(default_name)) + ->default_str(default_name); + + // add the transform only after default_val(), so the default does not + // go through the transform (CLI11 applies it at default_val() time). + opt->transform([choices = std::move(choices), + errmsg = mu_format("requires one of {}; default is {}", + choices_str, default_name)] + (const std::string& val) -> std::string { + if (const auto it = choices.find(val); it != choices.end()) + return std::to_string( + static_cast>(it->second)); + // normally rewritten by friendly_choice_error() + throw CLI::ValidationError{errmsg}; + }); + + friendly_errors.push_back( + {&sub, opt, mu_format("{} requires one of {}; default is {}", + opt->get_name(), choices_str, default_name)}); + return opt; +} + +/** The kind of number a numeric option accepts */ +enum struct Numeric { Positive, NonNegative }; + +/** + * Add an option to @p sub that takes a numeric value. + * + * Like CLI11's PositiveNumber/NonNegativeNumber checks, but with + * friendlier error messages, e.g. + * error: --maxnum requires a positive number + * + * @param sub command to add the option to + * @param name option name(s), e.g. "--maxnum,-n" + * @param value target for the parsed value + * @param help help text + * @param type_name name for the value in the help, e.g. "" + * @param kind the kind of number the option accepts + * + * @return the newly added option + */ +template +CLI::Option* +add_number_option(CLI::App& sub, const std::string& name, T& value, + const std::string& help, const std::string& type_name, + Numeric kind) +{ + const auto positive{kind == Numeric::Positive}; + + auto opt = sub.add_option(name, value, help) + ->type_name(type_name) + ->check(positive ? CLI::PositiveNumber : CLI::NonNegativeNumber); + + friendly_errors.push_back( + {&sub, opt, mu_format("{} requires a {} number", opt->get_name(), + positive ? "positive" : "non-negative")}); + return opt; +} + // transformers // Expand the path using wordexp -static const std::function ExpandPath = [](std::string filepath)->std::string { - if (auto&& res{expand_path(filepath)}; !res) +const std::function ExpandPath = [](const std::string& path)->std::string { + if (auto&& res{expand_path(path)}; !res) throw CLI::ValidationError{res.error().what()}; else return res.value(); }; // Canonicalize path -static const std::function CanonicalizePath = [](std::string filepath)->std::string { - return filepath = canonicalize_filename(filepath); +const std::function CanonicalizePath = [](const std::string& path)->std::string { + return canonicalize_filename(path); }; +} // end of anonymous namespace + /* * common */ @@ -234,7 +320,7 @@ static void sub_cfind(CLI::App& sub, Options& opts) { using Format = Options::Cfind::Format; - static constexpr InfoEnum FormatInfos = {{ + static constexpr auto FormatInfos = std::to_array>({ { Format::Plain, {"plain", "Plain output"} }, { Format::MuttAlias, {"mutt-alias", "Mutt alias"} }, { Format::MuttAddressBook, {"mutt-ab", "Mutt address book"}}, @@ -243,30 +329,23 @@ sub_cfind(CLI::App& sub, Options& opts) { Format::Bbdb, {"bbdb", "Emacs BBDB"}}, { Format::Csv, {"csv", "comma-separated values"}}, { Format::Json, {"json", "format as json array"}}, - }}; + }); - const auto fhelp = options_help(FormatInfos, Format::Plain); - const auto fmap = options_map(FormatInfos); - - sub.add_option("--format,-o", opts.cfind.format, - "Output format; one of " + fhelp) - ->type_name("") - ->default_str("plain") - ->default_val(Format::Plain) - ->transform(CLI::CheckedTransformer(fmap)); + add_choice_option(sub, "--format,-o", opts.cfind.format, + "Output format", "", + options_map(FormatInfos), options_names(FormatInfos), + "plain"); sub.add_option("pattern", opts.cfind.rx_pattern, "Regular expression pattern to match"); sub.add_flag("--personal,-p", opts.cfind.personal, "Only show 'personal' contacts"); - sub.add_option("--after", opts.cfind.after, - "Only show results after some timestamps") - ->type_name("") - ->check(CLI::PositiveNumber); - sub.add_option("--maxnum,-n", opts.cfind.maxnum, - "Maximum number of results") - ->type_name("") - ->check(CLI::PositiveNumber); + add_number_option(sub, "--after", opts.cfind.after, + "Only show results after some timestamps", + "", Numeric::NonNegative); + add_number_option(sub, "--maxnum,-n", opts.cfind.maxnum, + "Maximum number of results", + "", Numeric::Positive); } @@ -326,7 +405,7 @@ static void sub_find(CLI::App& sub, Options& opts) { using Format = Options::Find::Format; - static constexpr InfoEnum FormatInfos = {{ + static constexpr auto FormatInfos = std::to_array>({ { Format::Plain, {"plain", "Plain output"} }, @@ -345,7 +424,7 @@ sub_find(CLI::App& sub, Options& opts) { Format::Json2, {"json2", "more idiomatic JSON"} }, - }}; + }); sub.add_flag("--threads,-t", opts.find.threads, "Show message threads"); @@ -356,42 +435,32 @@ sub_find(CLI::App& sub, Options& opts) sub.add_flag("--analyze,-a", opts.find.analyze, "Analyze the query"); - const auto fhelp = options_help(FormatInfos, Format::Plain); - const auto fmap = options_map(FormatInfos); + add_choice_option(sub, "--format,-o", opts.find.format, + "Output format", "", + options_map(FormatInfos), options_names(FormatInfos), + "plain"); - sub.add_option("--format,-o", opts.find.format, - "Output format; one of " + fhelp) - ->type_name("") - ->default_str("plain") - ->default_val(Format::Plain) - ->transform(CLI::CheckedTransformer(fmap)); - - sub.add_option("--maxnum,-n", opts.find.maxnum, - "Maximum number of results") - ->type_name("") - ->check(CLI::PositiveNumber); + add_number_option(sub, "--maxnum,-n", opts.find.maxnum, + "Maximum number of results", + "", Numeric::Positive); sub.add_option("--fields,-f", opts.find.fields, "Fields to display") ->default_val("d f s"); std::unordered_map smap; - std::string sopts; + std::vector snames; field_for_each([&](auto&& field){ if (field.is_sortable()) { smap.emplace(std::string(field.name), field.id); smap.emplace(std::string(1, field.shortcut), field.id); - if (!sopts.empty()) - sopts += ", "; - sopts += mu_format("{}|{}", field.name, field.shortcut); + snames.emplace_back(mu_format("{}|{}", field.name, + field.shortcut)); } }); - sub.add_option("--sortfield,-s", opts.find.sortfield, - "Field to sort the results by; one of " + sopts) - ->type_name("") - ->default_str("date") - ->default_val(Field::Id::Date) - ->transform(CLI::CheckedTransformer(smap)); + add_choice_option(sub, "--sortfield,-s", opts.find.sortfield, + "Field to sort the results by", "", + std::move(smap), std::move(snames), "date"); sub.add_flag("--reverse,-z", opts.find.reverse, "Sort in descending order"); @@ -403,14 +472,18 @@ sub_find(CLI::App& sub, Options& opts) sub.add_flag("--clearlinks", opts.find.clearlinks, "Clear old links first"); sub.add_option("--linksdir", opts.find.linksdir, - "Use bookmarked query") + "Target directory for symlinks") ->type_name("") ->transform(ExpandPath, "expand linksdir path"); - sub.add_option("--summary-len", opts.find.summary_len, - "Use up to so many lines for the summary") - ->type_name("") - ->check(CLI::PositiveNumber); + add_number_option(sub, "--after", opts.find.after, + "Only show messages whose message file was changed " + "after some timestamp", + "", Numeric::NonNegative); + + add_number_option(sub, "--summary-len", opts.find.summary_len, + "Use up to so many lines for the summary", + "", Numeric::Positive); sub.add_option("--exec", opts.find.exec, "Command to execute on message file") @@ -469,7 +542,7 @@ sub_init(CLI::App& sub, Options& opts) // expand path. sub.add_option("--personal-address,--my-address", opts.init.personal_addresses, - "Personal e-mail address or regexp (can be used multiple titmes)") + "Personal e-mail address or regexp (can be used multiple times)") ->type_name("
"); sub.add_option("--ignored-address", opts.init.ignored_addresses, "Ignored e-mail address or regexp") @@ -479,8 +552,8 @@ sub_init(CLI::App& sub, Options& opts) "Maximum allowed message size in bytes"); sub.add_option("--batch-size", opts.init.batch_size, "Maximum size of database transaction"); - sub.add_option("--support-ngrams", opts.init.support_ngrams, - "Support CJK n-grams if for querying/indexing"); + sub.add_flag("--support-ngrams", opts.init.support_ngrams, + "Support CJK n-grams for querying/indexing"); sub.add_flag("--reinit", opts.init.reinit, "Re-initialize database with current settings") ->excludes("--maildir") @@ -669,7 +742,7 @@ static void sub_view(CLI::App& sub, Options& opts) { using Format = Options::View::Format; - static constexpr InfoEnum FormatInfos = {{ + static constexpr auto FormatInfos = std::to_array>({ { Format::Plain, {"plain", "Plain output"} }, @@ -679,24 +752,18 @@ sub_view(CLI::App& sub, Options& opts) { Format::Sexp, {"sexp", "S-expressions"} }, - }}; + }); - const auto fhelp = options_help(FormatInfos, Format::Plain); - const auto fmap = options_map(FormatInfos); - - sub.add_option("--format,-o", opts.view.format, - "Output format; one of " + fhelp) - ->type_name("") - ->default_str("plain") - ->default_val(Format::Plain) - ->transform(CLI::CheckedTransformer(fmap)); + add_choice_option(sub, "--format,-o", opts.view.format, + "Output format", "", + options_map(FormatInfos), options_names(FormatInfos), + "plain"); sub_crypto(sub, opts.view); - sub.add_option("--summary-len", opts.view.summary_len, - "Use up to so many lines for the summary") - ->type_name("") - ->check(CLI::PositiveNumber); + add_number_option(sub, "--summary-len", opts.view.summary_len, + "Use up to so many lines for the summary", + "", Numeric::Positive); sub.add_flag("--terminate", opts.view.terminate, "Insert form-feed after each message"); @@ -720,8 +787,8 @@ struct CommandInfo { setup_func_t setup_func{}; }; -static constexpr -AssocPairs SubCommandInfos= {{ +static constexpr auto SubCommandInfos = + std::to_array>({ { SubCommand::Add, { Category::NeedsWritableStore, "add", "Add messages to the database", sub_add} @@ -796,7 +863,9 @@ AssocPairs SubCommandInfos= {{ {Category::None, "view", "View specific messages", sub_view} }, - }}; + }); +static_assert(SubCommandInfos.size() == Options::SubCommandNum, + "SubCommandInfos must have an entry for each subcommand"); static ScriptInfos add_scripts(CLI::App& app, Options& opts) @@ -829,8 +898,10 @@ show_manpage(Options& opts, const std::string& name) GError* err{}; const auto cmd{mu_format("{} {}", *manprog, shell_quote(name))}; // run_command0 doesn't work here. - auto res = g_spawn_command_line_sync(cmd.c_str(), {}, {}, {}, &err); - if (!res) + int wait_status{}; + auto res = g_spawn_command_line_sync(cmd.c_str(), {}, {}, + &wait_status, &err); + if (!res || !g_spawn_check_wait_status(wait_status, &err)) return Err(Error::Code::Command, &err, "error running man command"); @@ -873,7 +944,6 @@ static void add_global_options(CLI::App& cli, Options& opts) { opts.nocolor = Options::default_no_color(); - errno = 0; cli.add_flag("-q,--quiet", opts.quiet, "Hide non-essential output"); cli.add_flag("-v,--verbose", opts.verbose, "Show verbose output"); @@ -892,6 +962,8 @@ Options::make(int argc, char *argv[]) Options opts{}; CLI::App app{"mu mail indexer/searcher " PACKAGE_VERSION, "mu"}; + friendly_errors.clear(); // entries refer to the previous app, if any. + app.description(R"(mu mail indexer/searcher Copyright (C) 2008-2025 Dirk-Jan C. Binnema @@ -973,7 +1045,8 @@ There is NO WARRANTY, to the extent permitted by law.)"); } catch (const CLI::CallForVersion&) { mu_println("version {}", PACKAGE_VERSION); } catch (const CLI::ParseError& pe) { - return Err(Error::Code::InvalidArgument, "{}", pe.what()); + return Err(Error::Code::InvalidArgument, "{}", + friendly_error(pe)); } catch (...) { return Err(Error::Code::Internal, "error parsing arguments"); } @@ -1026,12 +1099,12 @@ test_ids() #ifdef BUILD_TESTS enum struct TestEnum { A, B, C }; -constexpr AssocPairs -test_epairs = {{ +constexpr auto test_epairs = + std::to_array>({ {TestEnum::A, "a"}, {TestEnum::B, "b"}, {TestEnum::C, "c"}, -}}; +}); static constexpr Option to_name(TestEnum te) @@ -1053,6 +1126,103 @@ test_enum_pairs(void) g_assert_true(to_type("c").value() == TestEnum::C); } +static Result +test_make_options(std::vector args) +{ + std::vector argv; + argv.reserve(args.size()); + for (auto& arg: args) + argv.push_back(arg.data()); + + return Options::make(static_cast(argv.size()), argv.data()); +} + +static void +test_choice_option(void) +{ + // an explicit value + const auto explicit_fmt = + test_make_options({"mu", "find", "--format", "sexp", "x"}); + g_assert_true(!!explicit_fmt); + g_assert_true(explicit_fmt->sub_command == Options::SubCommand::Find); + g_assert_true(explicit_fmt->find.format == Options::Find::Format::Sexp); + + // the default + const auto default_fmt = test_make_options({"mu", "find", "x"}); + g_assert_true(!!default_fmt); + g_assert_true(default_fmt->find.format == Options::Find::Format::Plain); + g_assert_true(default_fmt->find.sortfield == Field::Id::Date); +} + +static void +test_choice_option_errors(void) +{ + constexpr auto errmsg = + "--format requires one of { json, json2, links, plain, sexp, xml }; " + "default is plain"; + + // bad value + const auto bad = test_make_options({"mu", "find", "--format=nope", "x"}); + g_assert_false(!!bad); + assert_equal(bad.error().what(), errmsg); + + // no value + const auto missing = test_make_options({"mu", "find", "x", "--format"}); + g_assert_false(!!missing); + assert_equal(missing.error().what(), errmsg); + + // another subcommand's --format gets its own choices + const auto view = test_make_options({"mu", "view", "--format=nope", "x"}); + g_assert_false(!!view); + assert_equal(view.error().what(), + "--format requires one of { html, plain, sexp }; " + "default is plain"); +} + +static void +test_sortfield_option(void) +{ + // by name and by shortcut + const auto by_name = + test_make_options({"mu", "find", "--sortfield=subject", "x"}); + g_assert_true(!!by_name); + g_assert_true(by_name->find.sortfield == Field::Id::Subject); + + const auto by_shortcut = test_make_options({"mu", "find", "-s", "s", "x"}); + g_assert_true(!!by_shortcut); + g_assert_true(by_shortcut->find.sortfield == Field::Id::Subject); + + const auto bad = test_make_options({"mu", "find", "-s", "nope", "x"}); + g_assert_false(!!bad); + g_assert_true(std::string{bad.error().what()} + .starts_with("--sortfield requires one of {")); +} + +static void +test_number_option(void) +{ + // valid values; --after=0 is allowed (non-negative) + const auto ok = test_make_options({"mu", "find", "--maxnum=10", + "--after=0", "x"}); + g_assert_true(!!ok); + g_assert_cmpuint(ok->find.maxnum.value(), ==, 10); + g_assert_cmpuint(ok->find.after.value(), ==, 0); + + // bad values + for (auto&& val: {"-5", "abc"}) { + const auto bad = test_make_options({"mu", "find", + mu_format("--maxnum={}", val), "x"}); + g_assert_false(!!bad); + assert_equal(bad.error().what(), + "--maxnum requires a positive number"); + } + + const auto negative = test_make_options({"mu", "find", "--after=-1", "x"}); + g_assert_false(!!negative); + assert_equal(negative.error().what(), + "--after requires a non-negative number"); +} + int main(int argc, char* argv[]) { @@ -1060,6 +1230,10 @@ main(int argc, char* argv[]) g_test_add_func("/options/ids", test_ids); g_test_add_func("/option/enum-pairs", test_enum_pairs); + g_test_add_func("/options/choice", test_choice_option); + g_test_add_func("/options/choice-errors", test_choice_option_errors); + g_test_add_func("/options/sortfield", test_sortfield_option); + g_test_add_func("/options/number", test_number_option); return g_test_run(); } diff --git a/mu/mu-options.hh b/mu/mu-options.hh index f9575ce4..86125e33 100644 --- a/mu/mu-options.hh +++ b/mu/mu-options.hh @@ -20,7 +20,7 @@ #ifndef MU_OPTIONS_HH__ #define MU_OPTIONS_HH__ -#include +#include #include #include #include @@ -29,7 +29,6 @@ #include #include -#include #include #include @@ -48,7 +47,6 @@ struct Options { */ bool quiet; /**< don't give any output */ bool debug; /**< log debug-level info */ - bool version; /**< request mu version */ bool log_stderr; /**< log to stderr */ bool nocolor; /**< don't use use ansi-colors */ bool verbose; /**< verbose output */ @@ -68,7 +66,7 @@ struct Options { __count__ }; static constexpr auto SubCommandNum = static_cast(SubCommand::__count__); - static constexpr std::array SubCommands = {{ + static constexpr auto SubCommands = std::to_array({ SubCommand::Add, SubCommand::Cfind, SubCommand::Extract, @@ -87,7 +85,9 @@ struct Options { SubCommand::Server, SubCommand::Verify, SubCommand::View - }}; + }); + static_assert(SubCommands.size() == SubCommandNum, + "SubCommands must list all subcommands"); Option sub_command; /**< The chosen sub-command, if any. */ @@ -152,15 +152,13 @@ struct Options { std::string bookmark; /**< use bookmark */ bool analyze; /**< analyze query */ - enum struct Format { Plain, Links, Xml, Json, Json2, Sexp, Exec }; + enum struct Format { Plain, Links, Xml, Json, Json2, Sexp }; Format format; /**< Output format */ std::string exec; /**< cmd to execute on matches */ bool skip_dups; /**< show only first with msg id */ bool include_related; /**< included related messages */ /**< for find and cind */ OptTStamp after; /**< only last seen after T */ - bool auto_retrieve; /**< assume we're online */ - bool decrypt; /**< try to decrypt the body */ StringVec query; /**< search query */ } find; diff --git a/mu/mu.cc b/mu/mu.cc index c0b2eb7d..bbe5dbef 100644 --- a/mu/mu.cc +++ b/mu/mu.cc @@ -29,8 +29,6 @@ #include "utils/mu-utils.hh" #include "utils/mu-logger.hh" -#include "mu-cmd.hh" - using namespace Mu; diff --git a/mu/tests/meson.build b/mu/tests/meson.build index fff02481..b0b92d7d 100644 --- a/mu/tests/meson.build +++ b/mu/tests/meson.build @@ -121,6 +121,15 @@ test('test-cmd-query', build_by_default: false, dependencies: [glib_dep, config_h_dep, lib_mu_dep])) +test('test-options', + executable('test-options', + '../mu-options.cc', + install: false, + build_by_default: false, + cpp_args: ['-DBUILD_TESTS', + '-DMU_SCRIPTS_DIR="'+ join_paths(datadir, 'mu', 'scripts') + '"'], + dependencies: [glib_dep, config_h_dep, cli11_dep, lib_mu_dep])) + gmime_test = executable( 'gmime-test', [ 'gmime-test.c'