From faf45cfec5d2161aa7ccafad41f6d5f4f54a18bf Mon Sep 17 00:00:00 2001
From: Patrick Storz <eduard.braun2@gmx.de>
Date: Wed, 25 Sep 2019 23:14:22 +0200
Subject: Extensions: Warn for duplicate page names and option names/values

This is easy to miss and causes confusing behavior
(e.g. wrong page/option selected)
---
 src/extension/prefdialog/parameter-notebook.cpp    | 13 +++++++++++++
 src/extension/prefdialog/parameter-notebook.h      |  4 ----
 src/extension/prefdialog/parameter-optiongroup.cpp | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+), 4 deletions(-)

(limited to 'src/extension')

diff --git a/src/extension/prefdialog/parameter-notebook.cpp b/src/extension/prefdialog/parameter-notebook.cpp
index a90935ca1..050c84f0d 100644
--- a/src/extension/prefdialog/parameter-notebook.cpp
+++ b/src/extension/prefdialog/parameter-notebook.cpp
@@ -15,6 +15,8 @@
 
 #include "parameter-notebook.h"
 
+#include <unordered_set>
+
 #include <gtkmm/box.h>
 #include <gtkmm/notebook.h>
 
@@ -133,6 +135,17 @@ ParamNotebook::ParamNotebook(Inkscape::XML::Node *xml, Inkscape::Extension::Exte
         g_warning("No (valid) pages for parameter '%s' in extension '%s'", _name, _extension->get_id());
     }
 
+    // check for duplicate page names
+    std::unordered_set<std::string> names;
+    for (auto child : _children) {
+        ParamNotebookPage *page = static_cast<ParamNotebookPage *>(child);
+        auto ret = names.emplace(page->_name);
+        if (!ret.second) {
+            g_warning("Duplicate page name ('%s') for parameter '%s' in extension '%s'.",
+                      page->_name, _name, _extension->get_id());
+        }
+    }
+
     // get value (initialize with value of first page if pref is empty)
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
     _value = prefs->getString(pref_name());
diff --git a/src/extension/prefdialog/parameter-notebook.h b/src/extension/prefdialog/parameter-notebook.h
index f5a313b49..b64e5c62a 100644
--- a/src/extension/prefdialog/parameter-notebook.h
+++ b/src/extension/prefdialog/parameter-notebook.h
@@ -54,10 +54,6 @@ private:
         std::string value_to_string() const override { return ""; };
     }; /* class ParamNotebookPage */
 
-    /** A table to store the pages with parameters for this notebook.
-      * This only gets created if there are pages in this notebook */
-    std::vector<ParamNotebookPage*> _pages;
-
 public:
     ParamNotebook(Inkscape::XML::Node *xml, Inkscape::Extension::Extension *ext);
 
diff --git a/src/extension/prefdialog/parameter-optiongroup.cpp b/src/extension/prefdialog/parameter-optiongroup.cpp
index 232c35315..954627e78 100644
--- a/src/extension/prefdialog/parameter-optiongroup.cpp
+++ b/src/extension/prefdialog/parameter-optiongroup.cpp
@@ -17,6 +17,8 @@
 
 #include "parameter-optiongroup.h"
 
+#include <unordered_set>
+
 #include <gtkmm/box.h>
 #include <gtkmm/comboboxtext.h>
 #include <gtkmm/radiobutton.h>
@@ -62,6 +64,22 @@ ParamOptionGroup::ParamOptionGroup(Inkscape::XML::Node *xml, Inkscape::Extension
         g_warning("No (valid) choices for parameter '%s' in extension '%s'", _name, _extension->get_id());
     }
 
+    // check for duplicate option texts and values
+    std::unordered_set<std::string> texts;
+    std::unordered_set<std::string> values;
+    for (auto choice : choices) {
+        auto ret1 = texts.emplace(choice->_text);
+        if (!ret1.second) {
+            g_warning("Duplicate option text ('%s') for parameter '%s' in extension '%s'.",
+                      choice->_text.c_str(), _name, _extension->get_id());
+        }
+        auto ret2 = values.emplace(choice->_value);
+        if (!ret2.second) {
+            g_warning("Duplicate option value ('%s') for parameter '%s' in extension '%s'.",
+                      choice->_value.c_str(), _name, _extension->get_id());
+        }
+    }
+
     // get value (initialize with value of first choice if pref is empty)
     Inkscape::Preferences *prefs = Inkscape::Preferences::get();
     _value = prefs->getString(pref_name());
-- 
cgit v1.2.3