Revert "improved code quality according to Minoru's advice"

This reverts commit 930605cac684e351e58c797fe4d376d50a16f5de.

10 files changed, 31 insertions, 38 deletions

diff --git a/include/utils.h b/include/utils.h
index ad649412..c86d8151 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -135,8 +135,7 @@ class utils {
 		static std::string make_title(const std::string& url);
 
 		static int run_interactively(
-				const std::string& command, const std::string& caller)
-				__attribute__ ((warn_unused_result));
+				const std::string& command, const std::string& caller);
 
 		static std::string getcwd();
 
diff --git a/include/view.h b/include/view.h
index 556a2aac..468d68ec 100644
--- a/include/view.h
+++ b/include/view.h
@@ -67,8 +67,8 @@ class view {
 		std::string select_tag();
 		std::string select_filter(const std::vector<filter_name_expr_pair>& filters);
 
-		bool open_in_browser(const std::string& url) __attribute__ ((warn_unused_result));;
-		bool open_in_pager(const std::string& filename) __attribute__ ((warn_unused_result));
+		bool open_in_browser(const std::string& url);
+		bool open_in_pager(const std::string& filename);
 
 		std::string get_filename_suggestion(const std::string& s);
 
diff --git a/src/controller.cpp b/src/controller.cpp
index 53b6dbe8..a712a776 100644
--- a/src/controller.cpp
+++ b/src/controller.cpp
@@ -1317,17 +1317,18 @@ void controller::edit_urls_file() {
 	v->push_empty_formaction();
 	stfl::reset();
 
-	/* We should not check this particular exit code.
-	 * Editors usually fail when closing+saving (and stay open).
-	 * (tested with vim, nano, gedit, geany)
-	 * (texmaker 5.0.1 and notepadqq 1.0.1 close without neither showing an
-	 * error nor saving but despite return 0)
-	 */
-	int unused __attribute__((unused));
-	unused = utils::run_interactively(cmdline, "controller::edit_urls_file");
+	int editor_exit_code = utils::run_interactively(cmdline, "controller::edit_urls_file");
+	if (editor_exit_code != 0) {
+		LOG(level::DEBUG, "utils::run_interactively: "
+			"The editor failed with error code %d - command was %s", editor_exit_code, cmdline);
+	}
 
 	v->pop_current_formaction();
 
+	if (editor_exit_code != 0) {
+		v->show_error(_("Editor failed to open url file!"));
+	}
+
 	reload_urls_file();
 }
 
@@ -1363,10 +1364,7 @@ std::string controller::bookmark(
 		if (is_interactive) {
 			v->push_empty_formaction();
 			stfl::reset();
-			int bookmark_cmd_exit_code = utils::run_interactively(cmdline, "controller::bookmark");
-			if (bookmark_cmd_exit_code != 0) {
-				return "The bookmark command failed!";
-			}
+			utils::run_interactively(cmdline, "controller::bookmark");
 			v->pop_current_formaction();
 			return "";
 		} else {
diff --git a/src/feedlist_formaction.cpp b/src/feedlist_formaction.cpp
index dd4a96af..eb484d9d 100644
--- a/src/feedlist_formaction.cpp
+++ b/src/feedlist_formaction.cpp
@@ -162,9 +162,8 @@ REDO:
 			if (feed) {
 				if (feed->rssurl().substr(0,6) != "query:") {
 					LOG(level::INFO, "feedlist_formaction: opening feed at position `%s': %s", feedpos, feed->link());
-					if (!v->open_in_browser(feed->link())) {
+					if (!v->open_in_browser(feed->link()))
 						v->show_error(_("Browser failed to open the link!"));
-					}
 
 				} else {
 					v->show_error(_("Cannot open query feeds in the browser!"));
diff --git a/src/itemlist_formaction.cpp b/src/itemlist_formaction.cpp
index 3b3fd724..06be6e85 100644
--- a/src/itemlist_formaction.cpp
+++ b/src/itemlist_formaction.cpp
@@ -84,6 +84,7 @@ void itemlist_formaction::process_operation(operation op, bool automatic, std::v
 		LOG(level::INFO, "itemlist_formaction: opening item at pos `%s'", itemposname);
 		if (itemposname.length() > 0 && visible_items.size() != 0) {
 			if (itempos < visible_items.size()) {
+
 				if (!v->open_in_browser(visible_items[itempos].first->link())) {
 					v->show_error(_("Browser failed to open the link!"));
 					break;
@@ -110,9 +111,9 @@ void itemlist_formaction::process_operation(operation op, bool automatic, std::v
 		LOG(level::INFO, "itemlist_formaction: opening item at pos `%s'", itemposname);
 		if (itemposname.length() > 0 && visible_items.size() != 0) {
 			if (itempos < visible_items.size()) {
-				if (!v->open_in_browser(visible_items[itempos].first->link())) {
+				if (!v->open_in_browser(visible_items[itempos].first->link()))
 					v->show_error(_("Browser failed to open the link!"));
-				}
+
 				invalidate(itempos);
 			}
 		} else {
diff --git a/src/itemview_formaction.cpp b/src/itemview_formaction.cpp
index 96062999..b006dcc3 100644
--- a/src/itemview_formaction.cpp
+++ b/src/itemview_formaction.cpp
@@ -211,9 +211,8 @@ void itemview_formaction::process_operation(operation op, bool automatic, std::v
 	case OP_OPENINBROWSER:
 		LOG(level::INFO, "view::run_itemview: starting browser");
 		v->set_status(_("Starting browser..."));
-		if (!v->open_in_browser(item->link())) {
+		if (!v->open_in_browser(item->link()))
 			v->show_error(_("Browser failed to open the link!"));
-		}
 		v->set_status("");
 		break;
 	case OP_BOOKMARK:
@@ -372,9 +371,8 @@ void itemview_formaction::process_operation(operation op, bool automatic, std::v
 		LOG(level::DEBUG, "itemview::run: OP_1 = %d op = %d idx = %u", OP_1, op, idx);
 		if (idx < links.size()) {
 			v->set_status(_("Starting browser..."));
-			if (!v->open_in_browser(links[idx].first)) {
+			if (!v->open_in_browser(links[idx].first))
 				v->show_error(_("Browser failed to open the link!"));
-			}
 			v->set_status("");
 		}
 	}
@@ -522,9 +520,8 @@ void itemview_formaction::finished_qna(operation op) {
 		sscanf(qna_responses[0].c_str(),"%u",&idx);
 		if (idx && idx-1 < links.size()) {
 			v->set_status(_("Starting browser..."));
-			if (!v->open_in_browser(links[idx-1].first)) {
+			if (!v->open_in_browser(links[idx-1].first))
 				v->show_error(_("Browser failed to open the link!"));
-			}
 			v->set_status("");
 		}
 	}
diff --git a/src/pb_controller.cpp b/src/pb_controller.cpp
index b95c18e9..e46594f3 100644
--- a/src/pb_controller.cpp
+++ b/src/pb_controller.cpp
@@ -371,9 +371,7 @@ void pb_controller::play_file(const std::string& file) {
 	cmdline.append(utils::replace_all(file,"\"", "\\\""));
 	cmdline.append("\"");
 	stfl::reset();
-	// Is the following exit code important?
-	int unused __attribute__((unused));
-	unused = utils::run_interactively(cmdline, "pb_controller::play_file");
+	utils::run_interactively(cmdline, "pb_controller::play_file");
 }
 
 
diff --git a/src/urlview_formaction.cpp b/src/urlview_formaction.cpp
index 7a2a9fda..c3b85cda 100644
--- a/src/urlview_formaction.cpp
+++ b/src/urlview_formaction.cpp
@@ -30,9 +30,8 @@ void urlview_formaction::process_operation(operation op, bool /* automatic */, s
 		if (posstr.length() > 0) {
 			unsigned int idx = utils::to_u(posstr, 0);
 			v->set_status(_("Starting browser..."));
-			if (!v->open_in_browser(links[idx].first)) {
+			if (!v->open_in_browser(links[idx].first))
 				v->show_error(_("Browser failed to open the link!"));
-			}
 			v->set_status("");
 		} else {
 			v->show_error(_("No link selected!"));
@@ -65,9 +64,8 @@ void urlview_formaction::process_operation(operation op, bool /* automatic */, s
 
 		if (idx < links.size()) {
 			v->set_status(_("Starting browser..."));
-			if (!v->open_in_browser(links[idx].first)) {
+			if (!v->open_in_browser(links[idx].first))
 				v->show_error(_("Browser failed to open the link!"));
-			}
 			v->set_status("");
 		}
 	}
diff --git a/src/utils.cpp b/src/utils.cpp
index 4f8bfed0..c9ad4e34 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -1102,11 +1102,6 @@ int utils::run_interactively(
 	} else if (status == 127) {
 		LOG(level::DEBUG, "%s: couldn't run shell", caller);
 	}
-	if (status != 0) {
-		LOG(level::DEBUG, "utils::run_interactively: "
-			"%s: cmd = %s, returned %d", caller, command, status);
-	}
-
 
 	return status;
 }
diff --git a/src/view.cpp b/src/view.cpp
index d84f1500..64c6cec0 100644
--- a/src/view.cpp
+++ b/src/view.cpp
@@ -333,6 +333,10 @@ bool view::open_in_pager(const std::string& filename) {
 	}
 	stfl::reset();
 	int pager_exit_code = utils::run_interactively(cmdline, "view::open_in_pager");
+	if (pager_exit_code != 0) {
+		LOG(level::DEBUG, "utils::run_interactively: "
+			"Starting the pager failed with error code %d - command was %s", pager_exit_code, cmdline);
+	}
 	pop_current_formaction();
 	return pager_exit_code == 0;
 }
@@ -361,6 +365,10 @@ bool view::open_in_browser(const std::string& url) {
 	}
 	stfl::reset();
 	int browser_exit_code = utils::run_interactively(cmdline, "view::open_in_browser");
+	if (browser_exit_code != 0) {
+		LOG(level::DEBUG, "utils::run_interactively: "
+			"Starting the browser failed with error code %d - command was %s", browser_exit_code, cmdline);
+	}
 	pop_current_formaction();
 
 	return browser_exit_code == 0;