From 42747070d3dd1ebefec800bdebb647176557f5bf Mon Sep 17 00:00:00 2001
From: Charles Dang <exodia339@gmail.com>
Date: Fri, 12 Jul 2019 17:21:26 +1100
Subject: [PATCH] Label Settings: code cleanup

---
 src/gui/dialogs/label_settings.cpp | 112 +++++++++++++++++------------
 src/gui/dialogs/label_settings.hpp |  29 ++++----
 2 files changed, 81 insertions(+), 60 deletions(-)

diff --git a/src/gui/dialogs/label_settings.cpp b/src/gui/dialogs/label_settings.cpp
index e611fc1dab43..c2a92f7cc3a4 100644
--- a/src/gui/dialogs/label_settings.cpp
+++ b/src/gui/dialogs/label_settings.cpp
@@ -15,82 +15,96 @@
 
 #include "gui/dialogs/label_settings.hpp"
 
-#include <vector>
-#include "utils/functional.hpp"
-#include "gettext.hpp"
 #include "display.hpp"
 #include "font/text_formatting.hpp"
-#include "map/label.hpp"
+#include "formatter.hpp"
+#include "formula/string_utils.hpp"
+#include "gettext.hpp"
 #include "gui/auxiliary/find_widget.hpp"
-#include "gui/widgets/styled_widget.hpp"
+#include "gui/widgets/label.hpp"
 #include "gui/widgets/listbox.hpp"
-#include "gui/widgets/window.hpp"
 #include "gui/widgets/settings.hpp"
+#include "gui/widgets/styled_widget.hpp"
 #include "gui/widgets/toggle_button.hpp"
-#include "gui/widgets/label.hpp"
-#include "formula/string_utils.hpp"
+#include "gui/widgets/window.hpp"
+#include "map/label.hpp"
 #include "team.hpp"
 
+#include <vector>
+
 namespace gui2
 {
 namespace dialogs
 {
 REGISTER_DIALOG(label_settings)
 
-label_settings::label_settings(display_context& dc) : viewer(dc) {
+label_settings::label_settings(display_context& dc)
+	: viewer_(dc)
+{
 	const std::vector<std::string>& all_categories = display::get_singleton()->labels().all_categories();
-	const std::vector<std::string>& hidden_categories = viewer.hidden_label_categories();
-
-	for(size_t i = 0; i < all_categories.size(); i++) {
-		all_labels[all_categories[i]] = true;
-		if(all_categories[i].substr(0,4) == "cat:")
-			labels_display[all_categories[i]] = all_categories[i].substr(4);
-		else if(all_categories[i] == "team")
-			labels_display[all_categories[i]] = _("Team Labels");
+	const std::vector<std::string>& hidden_categories = viewer_.hidden_label_categories();
+
+	for(const std::string& cat : all_categories) {
+		all_labels_[cat] = true;
+
 		// TODO: Translatable names for categories?
+		if(cat.substr(0, 4) == "cat:") {
+			labels_display_[cat] = cat.substr(4);
+		} else if(cat == "team") {
+			labels_display_[cat] = _("Team Labels");
+		}
 	}
-	for(size_t i = 0; i < hidden_categories.size(); i++) {
-		all_labels[hidden_categories[i]] = false;
+
+	for(const std::string& hidden_cat : hidden_categories) {
+		all_labels_[hidden_cat] = false;
 	}
-	for(size_t i = 0; i < dc.teams().size(); i++) {
-		const team& team = dc.teams()[i];
+
+	for(std::size_t i = 0; i < viewer_.teams().size(); i++) {
+		const team& team = viewer_.teams()[i];
 		const std::string label_cat_key = "side:" + std::to_string(i + 1);
+
 		if(team.hidden()) {
-			labels_display[label_cat_key] = "";
+			labels_display_[label_cat_key] = "";
 			continue;
+
 		}
+
 		std::string team_name = team.side_name();
 		if(team_name.empty()) {
 			team_name = team.user_team_name();
 		}
+
 		if(team_name.empty()) {
 			team_name = _("Unknown");
 		}
+
 		string_map subst;
 		subst["side_number"] = std::to_string(i + 1);
 		subst["name"] = team_name;
-		labels_display[label_cat_key] = VGETTEXT("Side $side_number ($name)", subst);
+		labels_display_[label_cat_key] = VGETTEXT("Side $side_number ($name)", subst);
 	}
 }
 
-void label_settings::pre_show(window& window) {
-	std::map<std::string, string_map> list_data;
+void label_settings::pre_show(window& window)
+{
 	listbox& cats_listbox = find_widget<listbox>(&window, "label_types", false);
-	for(const auto & label_entry : all_labels) {
+	std::map<std::string, string_map> list_data;
+
+	for(const auto& label_entry : all_labels_) {
 		const std::string& category = label_entry.first;
-		const bool& visible = label_entry.second;
+		const bool visible = label_entry.second;
 
-		std::string name = labels_display[category];
-		if(category.substr(0,5) == "side:") {
+		std::string name = labels_display_[category];
+		if(category.substr(0, 5) == "side:") {
+			// This means it's a hidden side, so don't show it.
 			if(name.empty()) {
-				// This means it's a hidden side, so don't show it.
 				continue;
 			}
-			int team = std::stoi(category.substr(5)) - 1;
-			color_t which_color = game_config::tc_info(viewer.teams()[team].color())[0];
-			std::ostringstream sout;
-			sout << font::span_color(which_color) << name << "</span>";
-			name = sout.str();
+
+			const int team = std::stoi(category.substr(5)) - 1;
+			const color_t tc = game_config::tc_info(viewer_.teams()[team].color())[0];
+
+			name = (formatter() << font::span_color(tc) << name << "</span>").str();
 		}
 
 		list_data["cat_name"]["label"] = name;
@@ -101,28 +115,32 @@ void label_settings::pre_show(window& window) {
 
 		connect_signal_notify_modified(status, std::bind(&label_settings::toggle_category, this, _1, category));
 
-		if(category.substr(0,5) == "side:") {
+		if(category.substr(0, 5) == "side:") {
 			label& cat_name = find_widget<label>(grid, "cat_name", false);
 			cat_name.set_use_markup(true);
 		}
 	}
 }
 
-bool label_settings::execute(display_context& dc) {
-	label_settings window(dc);
-	if(!window.show()) return false;
-	std::vector<std::string> hidden_categories;
-	for(auto lbl : window.all_labels) {
-		if(lbl.second == false) {
-			hidden_categories.push_back(lbl.first);
+void label_settings::post_show(window& window)
+{
+	if(get_retval() == retval::OK) {
+		std::vector<std::string> hidden_categories;
+
+		for(const auto& lbl : all_labels_) {
+			if(lbl.second == false) {
+				hidden_categories.push_back(lbl.first);
+			}
 		}
+
+		viewer_.hidden_label_categories().swap(hidden_categories);
 	}
-	dc.hidden_label_categories().swap(hidden_categories);
-	return true;
 }
 
-void label_settings::toggle_category(widget& box, std::string category) {
-	all_labels[category] = static_cast<toggle_button&>(box).get_value_bool();
+void label_settings::toggle_category(widget& box, const std::string& category)
+{
+	all_labels_[category] = static_cast<toggle_button&>(box).get_value_bool();
 }
+
 } // namespace dialogs
 } // namespace gui2
diff --git a/src/gui/dialogs/label_settings.hpp b/src/gui/dialogs/label_settings.hpp
index d08d7c4c77e7..09bbbf532821 100644
--- a/src/gui/dialogs/label_settings.hpp
+++ b/src/gui/dialogs/label_settings.hpp
@@ -14,29 +14,29 @@
 #pragma once
 
 #include "gui/dialogs/modal_dialog.hpp"
-#include <map>
+
 #include "display_context.hpp"
 #include "tstring.hpp"
 
+#include <map>
+
 namespace gui2
 {
 namespace dialogs
 {
-
-class label_settings : public modal_dialog {
+class label_settings : public modal_dialog
+{
 public:
 	label_settings(display_context& dc);
 
-	/**
-	 * The execute function.
-	 *
-	 * See @ref modal_dialog for more information.
-	 */
-	static bool execute(display_context& dc);
+	/** The execute function. See @ref modal_dialog for more information. */
+	DEFINE_SIMPLE_EXECUTE_WRAPPER(label_settings)
+
 private:
-	std::map<std::string, bool> all_labels;
-	std::map<std::string, t_string> labels_display;
-	display_context& viewer;
+	std::map<std::string, bool> all_labels_;
+	std::map<std::string, t_string> labels_display_;
+
+	display_context& viewer_;
 
 	/** Inherited from modal_dialog, implemented by REGISTER_DIALOG. */
 	virtual const std::string& window_id() const override;
@@ -44,8 +44,11 @@ class label_settings : public modal_dialog {
 	/** Inherited from modal_dialog. */
 	virtual void pre_show(window& window) override;
 
+	/** Inherited from modal_dialog. */
+	virtual void post_show(window& window) override;
+
 	/** Callback for toggling a checkbox state. */
-	void toggle_category(widget& box, std::string category);
+	void toggle_category(widget& box, const std::string& category);
 };
 } // namespace dialogs
 } // namespace gui2