GotoItem parsing regression (Close pekwm#161)

Fix regression in menu action parsing caused GotoItem default to NextItem instead.
pclouds · Mar 20, 2023 · 8f3e4fe · 8f3e4fe
1 parent 5fc7e8a
commit 8f3e4fe
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 6 deletions.
diff --git a/src/Config.cc b/src/Config.cc
@@ -84,11 +84,12 @@ static Util::StringTo<MouseEventType> mouse_event_map[] =
 static Util::StringTo<ActionType> menu_action_map[] =
 	{{"NEXTITEM", ACTION_MENU_NEXT},
 	 {"PREVITEM", ACTION_MENU_PREV},
+	 {"GOTOITEM", ACTION_MENU_GOTO},
 	 {"SELECT", ACTION_MENU_SELECT},
 	 {"ENTERSUBMENU", ACTION_MENU_ENTER_SUBMENU},
 	 {"LEAVESUBMENU", ACTION_MENU_LEAVE_SUBMENU},
 	 {"CLOSE", ACTION_CLOSE},
-	 {nullptr, ACTION_MENU_NEXT}};
+	 {nullptr, ACTION_NO}};
 
 static Util::StringTo<HarbourPlacement> harbour_placement_map[] =
 	{{"TOP", TOP},
@@ -875,12 +876,17 @@ Config::parseMenuAction(const std::string &action_string, Action &action)
 	// chop the string up separating the actions
 	if (Util::splitString(action_string, tok, " \t", 2)) {
 		action.setAction(Util::StringToGet(menu_action_map, tok[0]));
-		if (action.getAction() != ACTION_NO) {
-			return true;
+		if (action.getAction() == ACTION_MENU_GOTO) {
+			int idx = stoi_safe(tok[1], -1);
+			if (idx > 0) {
+				action.setParamI(0, idx - 1);
+			} else {
+				action.setAction(ACTION_NO);
+			}
 		}
 	}
 
-	return false;
+	return action.getAction() != ACTION_NO;
 }
 
 bool

diff --git a/src/lib/Compat.cc b/src/lib/Compat.cc
@@ -174,9 +174,20 @@ namespace std
 	}
 }
 
-
 #endif
 
+/**
+ * Variant of stoi where default value is returned on parse error.
+ */
+int
+stoi_safe(const std::string& str, int def)
+{
+	try {
+		return std::stoi(str);
+	} catch (std::invalid_argument&) {
+		return def;
+	}
+}
 
 #ifndef PEKWM_HAVE_STOF
 

diff --git a/src/lib/Compat.hh b/src/lib/Compat.hh
@@ -1,6 +1,6 @@
 //
 // Compat.hh for pekwm
-// Copyright (C) 2009-2020 Claes Nästén <[email protected]>
+// Copyright (C) 2009-2023 Claes Nästén <[email protected]>
 //
 // This program is licensed under the GNU GPL.
 // See the LICENSE file for more information.
@@ -101,6 +101,7 @@ namespace std
 }
 #endif
 
+int stoi_safe(const std::string& str, int def);
 
 #ifndef PEKWM_HAVE_STOF
 namespace std

diff --git a/test/test_Config.hh b/test/test_Config.hh
@@ -18,6 +18,8 @@ public:
 
 	virtual bool run_test(TestSpec spec, bool status);
 
+	void testParseMenuActionGotoItem(void);
+	void testParseMenuActionInvalid(void);
 	void testParseMoveResizeAction(void);
 };
 
@@ -34,10 +36,36 @@ TestConfig::~TestConfig(void)
 bool
 TestConfig::run_test(TestSpec spec, bool status)
 {
+	TEST_FN(spec, "parseMenuActionGotoItem", testParseMenuActionGotoItem());
+	TEST_FN(spec, "parseMenuActionInvalid", testParseMenuActionInvalid());
 	TEST_FN(spec, "parseMoveResizeAction", testParseMoveResizeAction());
 	return status;
 }
 
+void
+TestConfig::testParseMenuActionGotoItem(void)
+{
+	Action action;
+	ASSERT_EQUAL("parse", true,
+		     parseMenuAction("GotoItem 4", action));
+	ASSERT_EQUAL("action set", ACTION_MENU_GOTO, action.getAction());
+	// menu items start from 0, first item is adressed as 1
+	ASSERT_EQUAL("action param", 3, action.getParamI(0));
+
+	// not a number, result in no action
+	ASSERT_EQUAL("parse", false,
+		     parseMenuAction("GotoItem NaN", action));
+	ASSERT_EQUAL("action set", ACTION_NO, action.getAction());
+}
+
+void
+TestConfig::testParseMenuActionInvalid(void)
+{
+	Action action;
+	ASSERT_EQUAL("parse", false, parseMenuAction("Unknown", action));
+	ASSERT_EQUAL("action not set", ACTION_NO, action.getAction());
+}
+
 void
 TestConfig::testParseMoveResizeAction(void)
 {