Merge pull request #2137 from dennisschagt/fix/issue-2133

Allow selecting filter by name

3 files changed, 91 insertions, 86 deletions

diff --git a/src/feedlistformaction.cpp b/src/feedlistformaction.cpp
index ea216e49..326d34de 100644
--- a/src/feedlistformaction.cpp
+++ b/src/feedlistformaction.cpp
@@ -31,11 +31,11 @@ FeedListFormAction::FeedListFormAction(View* vv,
 	RegexManager& r)
 	: ListFormAction(vv, formstr, "feeds", cfg)
 	, zero_feedpos(false)
-	, apply_filter(false)
+	, filter_active(false)
 	, filterpos(0)
 	, set_filterpos(false)
 	, rxman(r)
-	, filters(f)
+	, filter_container(f)
 	, cache(cc)
 {
 	valid_cmds.push_back("tag");
@@ -423,30 +423,20 @@ REDO:
 	}
 	break;
 	case OP_SELECTFILTER:
-		if (filters.size() > 0) {
-			std::string newfilter;
+		if (filter_container.size() > 0) {
 			if (automatic && args->size() > 0) {
-				newfilter = (*args)[0];
-			} else {
-				newfilter = v->select_filter(
-						filters.get_filters());
-			}
-			if (newfilter != "") {
-				filterhistory.add_line(newfilter);
-				if (newfilter.length() > 0) {
-					if (!matcher.parse(newfilter)) {
-						v->get_statusline().show_error(strprintf::fmt(
-								_("Error: couldn't "
-									"parse filter "
-									"expression `%s': %s"),
-								newfilter,
-								matcher.get_parse_error()));
-					} else {
-						save_filterpos();
-						apply_filter = true;
-						do_redraw = true;
-					}
+				const std::string filter_name = (*args)[0];
+				const auto filter = filter_container.get_filter(filter_name);
+
+				if (filter.has_value()) {
+					apply_filter(filter.value());
+				} else {
+					v->get_statusline().show_error(strprintf::fmt(_("No filter found with name `%s'."),
+							filter_name));
 				}
+			} else {
+				const std::string filter_text = v->select_filter(filter_container.get_filters());
+				apply_filter(filter_text);
 			}
 		} else {
 			v->get_statusline().show_error(_("No filters defined."));
@@ -481,7 +471,7 @@ REDO:
 		}
 		break;
 	case OP_CLEARFILTER:
-		apply_filter = false;
+		filter_active = false;
 		do_redraw = true;
 		save_filterpos();
 		break;
@@ -605,7 +595,7 @@ void FeedListFormAction::update_visible_feeds(
 		feed->set_index(i + 1);
 		if ((tag == "" || feed->matches_tag(tag)) &&
 			(show_read || feed->unread_item_count() > 0) &&
-			(!apply_filter || matcher.matches(feed.get())) &&
+			(!filter_active || matcher.matches(feed.get())) &&
 			!feed->hidden()) {
 			visible_feeds.push_back(FeedPtrPosPair(feed, i));
 		}
@@ -951,7 +941,7 @@ void FeedListFormAction::update_form_title(unsigned int width)
 	fmt.register_fmt('u', std::to_string(count_unread_feeds()));
 	fmt.register_fmt('U', std::to_string(count_unread_articles()));
 	fmt.register_fmt('t', std::to_string(visible_feeds.size()));
-	fmt.register_fmt('F', apply_filter ? matcher.get_expression() : "");
+	fmt.register_fmt('F', filter_active ? matcher.get_expression() : "");
 
 	set_value("head", fmt.do_format(title_format, width));
 }
@@ -978,18 +968,7 @@ unsigned int FeedListFormAction::count_unread_articles()
 void FeedListFormAction::op_end_setfilter()
 {
 	std::string filtertext = qna_responses[0];
-	filterhistory.add_line(filtertext);
-	if (filtertext.length() > 0) {
-		if (!matcher.parse(filtertext)) {
-			v->get_statusline().show_error(strprintf::fmt(
-					_("Error: couldn't parse filter expression `%s': %s"),
-					filtertext, matcher.get_parse_error()));
-		} else {
-			save_filterpos();
-			apply_filter = true;
-			do_redraw = true;
-		}
-	}
+	apply_filter(filtertext);
 }
 
 void FeedListFormAction::op_start_search()
@@ -1118,4 +1097,23 @@ std::string FeedListFormAction::title()
 			static_cast<unsigned int>(visible_feeds.size()));
 }
 
+void FeedListFormAction::apply_filter(const std::string& filtertext)
+{
+	if (filtertext.empty()) {
+		return;
+	}
+
+	filterhistory.add_line(filtertext);
+	if (!matcher.parse(filtertext)) {
+		v->get_statusline().show_error(strprintf::fmt(
+				_("Error: couldn't parse filter expression `%s': %s"),
+				filtertext,
+				matcher.get_parse_error()));
+	} else {
+		save_filterpos();
+		filter_active = true;
+		do_redraw = true;
+	}
+}
+
 } // namespace newsboat
diff --git a/src/filtercontainer.cpp b/src/filtercontainer.cpp
index ea539993..87affb65 100644
--- a/src/filtercontainer.cpp
+++ b/src/filtercontainer.cpp
@@ -1,5 +1,7 @@
 #include "filtercontainer.h"
 
+#include <algorithm>
+
 #include "config.h"
 #include "confighandlerexception.h"
 #include "configparser.h"
@@ -52,4 +54,18 @@ void FilterContainer::dump_config(std::vector<std::string>& config_output) const
 	}
 }
 
+nonstd::optional<std::string> FilterContainer::get_filter(const std::string& name)
+{
+	const auto filter = std::find_if(filters.begin(),
+	filters.end(), [&](const FilterNameExprPair& pair) {
+		return pair.name == name;
+	});
+
+	if (filter != filters.end()) {
+		return filter->expr;
+	} else {
+		return {};
+	}
+}
+
 } // namespace newsboat
diff --git a/src/itemlistformaction.cpp b/src/itemlistformaction.cpp
index 5ac4459c..52b3800d 100644
--- a/src/itemlistformaction.cpp
+++ b/src/itemlistformaction.cpp
@@ -33,7 +33,7 @@ ItemListFormAction::ItemListFormAction(View* vv,
 	RegexManager& r)
 	: ListFormAction(vv, formstr, "items", cfg)
 	, old_itempos(-1)
-	, apply_filter(false)
+	, filter_active(false)
 	, pos(0)
 	, set_filterpos(false)
 	, filterpos(0)
@@ -42,7 +42,7 @@ ItemListFormAction::ItemListFormAction(View* vv,
 	, invalidation_mode(InvalidationMode::NONE)
 	, listfmt(&rxman, "articlelist")
 	, rsscache(cc)
-	, filters(f)
+	, filter_container(f)
 {
 	register_format_styles();
 }
@@ -492,7 +492,7 @@ bool ItemListFormAction::process_operation(Operation op,
 				}
 				rsscache->mark_items_read_by_guid(guids);
 
-				if (apply_filter) {
+				if (filter_active) {
 					// We're only viewing a subset of items, so mark them off one by one.
 					v->get_ctrl()->mark_all_read(guids);
 				} else {
@@ -617,36 +617,21 @@ bool ItemListFormAction::process_operation(Operation op,
 		v->get_ctrl()->edit_urls_file();
 		break;
 	case OP_SELECTFILTER:
-		if (filters.size() > 0) {
+		if (filter_container.size() > 0) {
 			std::string newfilter;
-			if (automatic) {
-				if (args->size() > 0) {
-					newfilter = (*args)[0];
+			if (automatic && args->size() > 0) {
+				const std::string filter_name = (*args)[0];
+				const auto filter = filter_container.get_filter(filter_name);
+
+				if (filter.has_value()) {
+					apply_filter(filter.value());
+				} else {
+					v->get_statusline().show_error(strprintf::fmt(_("No filter found with name `%s'."),
+							filter_name));
 				}
 			} else {
-				newfilter = v->select_filter(
-						filters.get_filters());
-				LOG(Level::DEBUG,
-					"ItemListFormAction::run: newfilters "
-					"= %s",
-					newfilter);
-			}
-			if (newfilter != "") {
-				filterhistory.add_line(newfilter);
-				if (newfilter.length() > 0) {
-					if (!matcher.parse(newfilter)) {
-						v->get_statusline().show_error(strprintf::fmt(
-								_("Error: couldn't "
-									"parse filter "
-									"expression `%s': %s"),
-								newfilter,
-								matcher.get_parse_error()));
-					} else {
-						apply_filter = true;
-						invalidate_list();
-						save_filterpos();
-					}
-				}
+				const std::string filter_text = v->select_filter(filter_container.get_filters());
+				apply_filter(filter_text);
 			}
 		} else {
 			v->get_statusline().show_error(_("No filters defined."));
@@ -668,7 +653,7 @@ bool ItemListFormAction::process_operation(Operation op,
 		}
 		break;
 	case OP_CLEARFILTER:
-		apply_filter = false;
+		filter_active = false;
 		invalidate_list();
 		save_filterpos();
 		break;
@@ -853,20 +838,7 @@ void ItemListFormAction::finished_qna(Operation op)
 void ItemListFormAction::qna_end_setfilter()
 {
 	std::string filtertext = qna_responses[0];
-	filterhistory.add_line(filtertext);
-
-	if (filtertext.length() > 0) {
-		if (!matcher.parse(filtertext)) {
-			v->get_statusline().show_error(strprintf::fmt(
-					_("Error: couldn't parse filter expression `%s': %s"),
-					filtertext, matcher.get_parse_error()));
-			return;
-		}
-
-		apply_filter = true;
-		invalidate_list();
-		save_filterpos();
-	}
+	apply_filter(filtertext);
 }
 
 void ItemListFormAction::qna_end_editflags()
@@ -944,7 +916,7 @@ void ItemListFormAction::do_update_visible_items()
 	for (const auto& item : items) {
 		item->set_index(i + 1);
 		if ((show_read || item->unread()) &&
-			(!apply_filter || matcher.matches(item.get()))) {
+			(!filter_active || matcher.matches(item.get()))) {
 			new_visible_items.push_back(ItemPtrPosPair(item, i));
 		}
 		i++;
@@ -1193,7 +1165,7 @@ FmtStrFormatter ItemListFormAction::setup_head_formatter(const std::string& s,
 
 	fmt.register_fmt('U', utils::censor_url(url));
 
-	fmt.register_fmt('F', apply_filter ? matcher.get_expression() : "");
+	fmt.register_fmt('F', filter_active ? matcher.get_expression() : "");
 
 	return fmt;
 }
@@ -1617,4 +1589,23 @@ void ItemListFormAction::handle_op_saveall()
 	}
 }
 
+void ItemListFormAction::apply_filter(const std::string& filtertext)
+{
+	if (filtertext.empty()) {
+		return;
+	}
+
+	filterhistory.add_line(filtertext);
+	if (!matcher.parse(filtertext)) {
+		v->get_statusline().show_error(strprintf::fmt(
+				_("Error: couldn't parse filter expression `%s': %s"),
+				filtertext,
+				matcher.get_parse_error()));
+	} else {
+		save_filterpos();
+		filter_active = true;
+		invalidate_list();
+	}
+}
+
 } // namespace newsboat