labels: improve logging for import

Clear up the code a bit using the new logging functions.

Don't log a failure for "plan A" as an error, it just scares user unnecessarily.

3 files changed, 48 insertions, 36 deletions

diff --git a/lib/message/mu-labels.cc b/lib/message/mu-labels.cc
index 87b92e3..9b90aba 100644
--- a/lib/message/mu-labels.cc
+++ b/lib/message/mu-labels.cc
@@ -30,7 +30,8 @@ Mu::Labels::validate_label(const std::string &label)
 	if (label.empty())
 		return Err(Error{Error::Code::InvalidArgument,
 				 "labels cannot be empty"});
-	else if (!g_utf8_validate(label.c_str(), label.size(), {})) // perhaps put hex in err str?
+	else if (!g_utf8_validate(label.c_str(), label.size(), {}))
+		// perhaps put hex in err str?
 		return Err(Error{Error::Code::InvalidArgument,
 				"labels must be valid UTF-8"});
 
@@ -43,7 +44,7 @@ Mu::Labels::validate_label(const std::string &label)
 		return Err(Error{Error::Code::InvalidArgument,
 				 "labels cannot start with '+' or '-' ({})", label});
 
-	for (auto cur = cstr; cur && *cur; cur = g_utf8_next_char(cur)) {
+	for (auto cur = cstr; cur && *cur; cur = g_utf8_next_char(cur)) { // NOLINT
 
 		const gunichar uc = g_utf8_get_char(cur);
 		if (g_unichar_isalnum(uc))
@@ -117,7 +118,7 @@ Mu::Labels::updated_labels(const LabelVec& labels, const DeltaLabelVec& deltas)
 
 	// First, the delta; put in a set for uniqueness; and use a special
 	// comparison operator so "add" and "remove" deltas are considered "the same"
-	// for the set; then fill the set from the end of the deltas vec to the begining,
+	// for the set; then fill the set from the end of the deltas vec to the beginning,
 	// so "the last one wins", as we want.
 
 	// only one change per label, last one wins
diff --git a/lib/mu-store-labels.cc b/lib/mu-store-labels.cc
index a08f3b4..c3f0465 100644
--- a/lib/mu-store-labels.cc
+++ b/lib/mu-store-labels.cc
@@ -76,6 +76,7 @@ Mu::export_labels(const Store& store, const std::string& query, Option<std::stri
 
 	auto output_res = export_output(path);
 	if (!output_res)
+
 		return Err(std::move(output_res.error()));
 
 	auto&[output, output_path] = *output_res;
@@ -97,43 +98,45 @@ Mu::export_labels(const Store& store, const std::string& query, Option<std::stri
 	return Ok(std::move(output_path));
 }
 
-static void
-log_import(bool quiet, bool verbose, const std::string& msg, bool is_err=false)
-{
-	if (is_err)
-		mu_debug("{}", msg);
-	else
-		mu_warning("{}", msg);
-
-	if (is_err && !quiet)
-		mu_printerrln("{}", msg);
-	else if (verbose)
-		mu_println("{}", msg);
+namespace Levels {
+using Level = size_t;
+constexpr Level quiet = 1 << 0;
+constexpr Level verbose = 1 << 1;
+constexpr Level error = 1 << 0;
 }
 
-static void
-log_import_err(bool quiet, bool verbose, const std::string& msg)
-{
-	log_import(quiet, verbose, msg, true);
-}
+using Level = Levels::Level;
 
+template<typename...T>
+static void output(Level level,
+		   fmt::format_string<T...> frm, T&&... args) noexcept {
+
+	GLogLevelFlags lflags = level & Levels::error ?
+		G_LOG_LEVEL_WARNING : G_LOG_LEVEL_DEBUG;
+	mu_log(lflags, frm, std::forward<T>(args)...);
+
+	if ((level & Levels::error))
+		mu_printerrln(frm, std::forward<T>(args)... );
+	else if ((level & Levels::verbose))
+		mu_println(frm, std::forward<T>(args)... );
+
+}
 
 static Result<QueryResults>
-log_import_get_matching(Mu::Store& store, const std::string& query, int max=1)
+import_get_matching(Mu::Store& store, const std::string& query, int max=1)
 {
 	if (auto qres = store.run_query(query, {}, {}, max); !qres)
 		return Err(std::move(qres.error()));
 	else if (qres->empty())
 		return Err(Error{Error::Code::Query,
-				"no matching messages for {}", query});
+				"no match for or '{}'", query});
 	else
 		return Ok(std::move(*qres));
 }
 
 
 static void
-import_labels_for_message(Mu::Store& store,
-			  bool dry_run, bool quiet, bool verbose,
+import_labels_for_message(Mu::Store& store, bool dry_run, Level level,
 			  const std::string& path, const std::string& msgid,
 			  const std::vector<std::string> labels)
 {
@@ -147,33 +150,38 @@ import_labels_for_message(Mu::Store& store,
 
 	const auto qres = [&]()->Result<QueryResults>{
 		// plan A: match by path
-		if (auto qres_a{log_import_get_matching(store, "path:" + path)}; !qres_a) {
-			log_import_err(quiet, verbose, mu_format("failed to find by path: {}; try with message-id",
-				       qres_a.error().what()));
+		if (auto qres_a{import_get_matching(store, "path:" + path)}; !qres_a) {
+			output(level, "path '{}' does not match; try message-id",
+			       qres_a.error().what());
 			// plan B: try the message-id
-			return log_import_get_matching(store, "msgid:" + msgid, -1/*all matching*/);
+			auto qres_b{import_get_matching(store, "msgid:" + msgid, -1/*all matching*/)};
+			if (!qres_b) { // plan-B failed too?
+				output(level | Levels::error,
+				       "import failed: cannot find message by path '{}' or message-id '{}'",
+				       path, msgid);
+			}
+			return qres_b;
 		} else
 			return qres_a;
 	}();
 
 	// neither plan a or b worked? we have to give up...
 	if (!qres) {
-		log_import_err(quiet, verbose, qres.error().what());
 		return;
 	}
 
+	mu_println("{} matches", qres->size());
+
 	// we have match(es)!
 	for (auto&& item: *qres) {
 		auto msg{*item.message()};
 		if (dry_run )
-			mu_println("labels: would apply label '{}' to {}", join(labels, ","), path);
+			output(level, "{}: would have applied labels {}", msg.path(), join(labels, ","));
 		else if (const auto res = store.update_labels(msg, delta_labels); !res)
-			log_import_err(quiet, verbose,
-				       mu_format("failed to update labels for {}: {}",
-						 msg.path(), res.error().what()));
+			output(level | Levels::error, "failed to update labels for {}: {}",
+			       msg.path(), res.error().what());
 		else
-			log_import(quiet, verbose,
-				   mu_format("applied labels {} to {}", join(labels, ","), path));
+			output(level, "{}: applied labels {}", msg.path(), join(labels, ","));
 	}
 }
 
@@ -190,6 +198,8 @@ Mu::import_labels(Mu::Store& store, const std::string& path, bool dry_run, bool
 	std::string current_path, current_msgid;
 	std::vector<std::string> current_labels;
 
+	const Level level{(quiet ? Levels::quiet : 0) | (verbose ? Levels::verbose : 0)};
+
 	while (std::getline(input, line)) {
 
 		if (line.find(path_key) ==  0)
@@ -199,7 +209,7 @@ Mu::import_labels(Mu::Store& store, const std::string& path, bool dry_run, bool
 		else if (line.find(labels_key) == 0) {
 			current_labels = split(line.substr(labels_key.length()), ',');
 			if (!current_labels.empty())
-				import_labels_for_message(store, dry_run, quiet, verbose,
+				import_labels_for_message(store, dry_run, level,
 							current_path, current_msgid,
 							current_labels);
 			current_path.clear();
diff --git a/lib/mu-store-labels.hh b/lib/mu-store-labels.hh
index 919e9de..4037020 100644
--- a/lib/mu-store-labels.hh
+++ b/lib/mu-store-labels.hh
@@ -43,6 +43,7 @@ class Store; // fwd declaration
 Result<std::string> export_labels(const Store& store,
 				  const std::string& query="",
 				  Option<std::string> path={});
+
 /**
  * Import labels from a file
  *