Switch Inkscape::Extension to use InxWidgets instead of InxParameters

8 files changed, 106 insertions, 60 deletions

diff --git a/src/extension/effect.cpp b/src/extension/effect.cpp
index a9088f88d..976c26192 100644
--- a/src/extension/effect.cpp
+++ b/src/extension/effect.cpp
@@ -239,7 +239,7 @@ Effect::prefs (Inkscape::UI::View::View * doc)
         return true;
     }
 
-    if (param_visible_count() == 0) {
+    if (widget_visible_count() == 0) {
         effect(doc);
         return true;
     }
diff --git a/src/extension/effect.h b/src/extension/effect.h
index d2b5606f2..2312bb838 100644
--- a/src/extension/effect.h
+++ b/src/extension/effect.h
@@ -73,7 +73,7 @@ class Effect : public Extension {
                     _elip_name(nullptr) {
                 /* No clue why, but this is required */
                 this->set_default_sensitive(true);
-                if (_showPrefs && effect != nullptr && effect->param_visible_count() != 0) {
+                if (_showPrefs && effect != nullptr && effect->widget_visible_count() != 0) {
                     _elip_name = g_strdup_printf("%s...", _(name));
                     set_name(_elip_name);
                 }
diff --git a/src/extension/extension.cpp b/src/extension/extension.cpp
index 83bfc7a8a..481283d97 100644
--- a/src/extension/extension.cpp
+++ b/src/extension/extension.cpp
@@ -16,22 +16,26 @@
  * Released under GNU GPL v2+, read the file 'COPYING' for more information.
  */
 
+#include "extension.h"
+#include "implementation/implementation.h"
+
+#include <glibmm/i18n.h>
 #include <gtkmm/box.h>
-#include <gtkmm/label.h>
 #include <gtkmm/frame.h>
 #include <gtkmm/grid.h>
-
-#include <glibmm/i18n.h>
-#include "inkscape.h"
-#include "extension/implementation/implementation.h"
-#include "extension.h"
+#include <gtkmm/label.h>
 
 #include "db.h"
 #include "dependency.h"
+#include "inkscape.h"
 #include "timer.h"
-#include "prefdialog/parameter.h"
+
 #include "io/resource.h"
 
+#include "prefdialog/parameter.h"
+#include "prefdialog/widget.h"
+
+
 namespace Inkscape {
 namespace Extension {
 
@@ -100,10 +104,10 @@ Extension::Extension (Inkscape::XML::Node *in_repr, Implementation::Implementati
             } else {
                 throw extension_no_name();
             }
-        } else if (!strcmp(chname, "param")) {
-            InxParameter *param = InxParameter::make(child_repr, this);
-            if (param) {
-                parameters.push_back(param);
+        } else if (InxWidget::is_valid_widget_name(chname)) {
+            InxWidget *widget = InxWidget::make(child_repr, this);
+            if (widget) {
+                _widgets.push_back(widget);
             }
         } else if (!strcmp(chname, "dependency")) {
             _deps.push_back(new Dependency(child_repr));
@@ -155,9 +159,8 @@ Extension::~Extension ()
     delete timer;
     timer = nullptr;
 
-    // delete parameters:
-    for (auto parameter : parameters) {
-        delete parameter;
+    for (auto widget : _widgets) {
+        delete widget;
     }
 
     for (auto & _dep : _deps) {
@@ -410,17 +413,20 @@ InxParameter *Extension::get_param(const gchar *name)
     if (name == nullptr) {
         throw Extension::param_not_exist();
     }
-    if (this->parameters.empty()) {
+    if (this->_widgets.empty()) {
         throw Extension::param_not_exist();
     }
 
-    for( auto param:this->parameters) {
-        if (!strcmp(param->name(), name)) {
-            return param;
-        } else {
-            InxParameter * subparam = param->get_param(name);
-            if (subparam) {
-                return subparam;
+    for(auto widget : _widgets) {
+        InxParameter *parameter = dynamic_cast<InxParameter *>(widget); // filter InxParameters from InxWidgets
+        if (parameter) {
+            if (!strcmp(parameter->name(), name)) {
+                return parameter;
+            } else {
+                InxParameter *subparam = parameter->get_param(name);
+                if (subparam) {
+                    return subparam;
+                }
             }
         }
     }
@@ -714,31 +720,35 @@ public:
     };
 };
 
-/** \brief  A function to automatically generate a GUI using the parameters
+/** \brief  A function to automatically generate a GUI from the extensions' widgets
     \return Generated widget
 
-    This function just goes through each parameter, and calls it's 'get_widget'
-    function to get each widget.  Then, each of those is placed into
-    a Gtk::VBox, which is then returned to the calling function.
+    This function just goes through each widget, and calls it's 'get_widget'.
+    Then, each of those is placed into a Gtk::VBox, which is then returned to the calling function.
 
     If there are no visible parameters, this function just returns NULL.
-    If all parameters are gui_hidden = true NULL is returned as well.
 */
 Gtk::Widget *
 Extension::autogui (SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal)
 {
-    if (!_gui || param_visible_count() == 0) return nullptr;
+    if (!_gui || widget_visible_count() == 0) {
+        return nullptr;
+    }
 
     AutoGUI * agui = Gtk::manage(new AutoGUI());
     agui->set_border_width(InxParameter::GUI_BOX_MARGIN);
     agui->set_spacing(InxParameter::GUI_BOX_SPACING);
 
-    //go through the list of parameters to see if there are any non-hidden ones
-    for (auto param:parameters) {
-        if (param->get_hidden()) continue; //Ignore hidden parameters
-        Gtk::Widget * widg = param->get_widget(doc, node, changeSignal);
-        gchar const * tip = param->get_tooltip();
-        int indent = param->get_indent();
+    // go through the list of widgets and add the all non-hidden ones
+    for (auto widget : _widgets) {
+        if (widget->get_hidden()) {
+            continue;
+        }
+
+        Gtk::Widget *widg = widget->get_widget(doc, node, changeSignal);
+        gchar const *tip = widget->get_tooltip();
+        int indent = widget->get_indent();
+
         agui->addWidget(widg, tip, indent);
     }
 
@@ -754,8 +764,11 @@ Extension::autogui (SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<voi
 void
 Extension::paramListString (std::list <std::string> &retlist)
 {
-    for(auto param:parameters) {
-        param->string(retlist);
+    for (auto widget : _widgets) {
+        InxParameter *parameter = dynamic_cast<InxParameter *>(widget); // filter InxParameters from InxWidgets
+        if (parameter) {
+            parameter->string(retlist);
+        }
     }
 
     return;
@@ -816,11 +829,13 @@ Extension::get_params_widget()
     return retval;
 }
 
-unsigned int Extension::param_visible_count ( )
+unsigned int Extension::widget_visible_count ( )
 {
     unsigned int _visible_count = 0;
-    for (auto param:parameters) {
-        if (!param->get_hidden()) _visible_count++;
+    for (auto widget : _widgets) {
+        if (!widget->get_hidden()) {
+            _visible_count++;
+        }
     }
     return _visible_count;
 }
diff --git a/src/extension/extension.h b/src/extension/extension.h
index 0868429b4..084b8e76d 100644
--- a/src/extension/extension.h
+++ b/src/extension/extension.h
@@ -18,10 +18,14 @@
 #include <ostream>
 #include <fstream>
 #include <vector>
-#include <glibmm/ustring.h>
 #include "xml/repr.h"
+
 #include <sigc++/signal.h>
 
+namespace Glib {
+    class ustring;
+}
+
 namespace Gtk {
 	class Grid;
 	class VBox;
@@ -70,6 +74,11 @@ namespace Gtk {
 class SPDocument;
 
 namespace Inkscape {
+
+namespace XML {
+class Node;
+}
+
 namespace Extension {
 
 class ExecutionEnv;
@@ -77,6 +86,7 @@ class Dependency;
 class ExpirationTimer;
 class ExpirationTimer;
 class InxParameter;
+class InxWidget;
 
 namespace Implementation
 {
@@ -138,24 +148,12 @@ public:
 
 /* Parameter Stuff */
 private:
-    std::vector<InxParameter *> parameters; /**< A table to store the parameters for this extension.
-                                                 This only gets created if there are parameters in this extension */
+    std::vector<InxWidget *> _widgets; /**< A list of widgets for this extension. */
 
 public:
-    /** \brief  A function to get the number of parameters that
-                the extension has.
-        \return The number of parameters. */
-    unsigned int param_count ( ) { return parameters.size(); };
-    /** \brief  A function to get the number of parameters that
-                are visible to the user that the extension has.
-        \return The number of visible parameters.
-
-        \note Currently this just calls param_count as visible isn't implemented
-              but in the future it'll do something different.  Please call
-              the appropriate function in code so that it'll work in the
-              future.
-    */
-    unsigned int param_visible_count ( );
+    /** \brief  A function to get the number of visible parameters of the extension.
+        \return The number of visible parameters. */
+    unsigned int widget_visible_count ( );
 
 public:
     /** An error class for when a parameter is looked for that just
diff --git a/src/extension/implementation/implementation.cpp b/src/extension/implementation/implementation.cpp
index 949451293..36cd299b7 100644
--- a/src/extension/implementation/implementation.cpp
+++ b/src/extension/implementation/implementation.cpp
@@ -37,7 +37,7 @@ Implementation::prefs_output(Inkscape::Extension::Output *module) {
 
 Gtk::Widget *Implementation::prefs_effect(Inkscape::Extension::Effect *module, Inkscape::UI::View::View * view, sigc::signal<void> * changeSignal, ImplementationDocumentCache * /*docCache*/)
 {
-    if (module->param_visible_count() == 0) {
+    if (module->widget_visible_count() == 0) {
         return nullptr;
     }
 
diff --git a/src/extension/prefdialog/parameter.h b/src/extension/prefdialog/parameter.h
index 7db7b9a3f..c7e6acb86 100644
--- a/src/extension/prefdialog/parameter.h
+++ b/src/extension/prefdialog/parameter.h
@@ -108,7 +108,7 @@ public:
      */
     static InxParameter *make(Inkscape::XML::Node *in_repr, Inkscape::Extension::Extension *in_ext);
 
-    const gchar *get_tooltip() const { return _description; }
+    const gchar *get_tooltip() const override { return _description; }
 
     virtual void string(std::list <std::string> &list) const;
 
diff --git a/src/extension/prefdialog/widget.cpp b/src/extension/prefdialog/widget.cpp
index 1297edb1c..ae85b9fbd 100644
--- a/src/extension/prefdialog/widget.cpp
+++ b/src/extension/prefdialog/widget.cpp
@@ -10,9 +10,12 @@
  * Released under GNU GPL v2+, read the file 'COPYING' for more information.
  */
 
+#include "parameter.h"
 #include "widget.h"
 #include "widget-label.h"
 
+#include <algorithm>
+
 #include <sigc++/sigc++.h>
 
 #include "extension/extension.h"
@@ -28,11 +31,20 @@ InxWidget *InxWidget::make(Inkscape::XML::Node *in_repr, Inkscape::Extension::Ex
     InxWidget *widget = nullptr;
 
     const char *name = in_repr->name();
+    if (!strncmp(name, INKSCAPE_EXTENSION_NS_NC, strlen(INKSCAPE_EXTENSION_NS_NC))) {
+        name += strlen(INKSCAPE_EXTENSION_NS);
+    }
+    if (name[0] == '_') { // allow leading underscore in tag names for backwards-compatibility
+        name++;
+    }
+
     if (!name) {
         // we can't create a widget without name
         g_warning("InxWidget without name in extension '%s'.", in_ext->get_id());
     } else if (!strcmp(name, "description")) {
         widget = new WidgetLabel(in_repr, in_ext);
+    } else if (!strcmp(name, "param")) {
+        widget = InxParameter::make(in_repr, in_ext);
     } else {
         g_warning("Unknown widget name ('%s') in extension '%s'", name, in_ext->get_id());
     }
@@ -41,6 +53,19 @@ InxWidget *InxWidget::make(Inkscape::XML::Node *in_repr, Inkscape::Extension::Ex
     return widget;
 }
 
+bool InxWidget::is_valid_widget_name(const char *name)
+{
+    // keep in sync with names supported in InxWidget::make() above
+    static const std::vector<std::string> valid_names = {"description", "param"};
+
+    if (std::find(valid_names.begin(), valid_names.end(), name) != valid_names.end()) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
+
 InxWidget::InxWidget (Inkscape::XML::Node *in_repr, Inkscape::Extension::Extension *ext)
     : _extension(ext)
 {
diff --git a/src/extension/prefdialog/widget.h b/src/extension/prefdialog/widget.h
index 31fe957f1..95285e2d6 100644
--- a/src/extension/prefdialog/widget.h
+++ b/src/extension/prefdialog/widget.h
@@ -14,6 +14,7 @@
 #define SEEN_INK_EXTENSION_WIDGET_H
 
 #include <string>
+#include <vector>
 
 #include <sigc++/sigc++.h>
 #include <glibmm/ustring.h>
@@ -57,8 +58,14 @@ public:
      */
     static InxWidget *make(Inkscape::XML::Node *in_repr, Inkscape::Extension::Extension *in_ext);
 
+    /** Checks if name is a valid widget name, i.e. a widget can be constructed from it using make() */
+    static bool is_valid_widget_name(const char *name);
+
+
     virtual Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal);
 
+    virtual const gchar *get_tooltip() const { return nullptr; } // tool-tips are exclusive to InxParameters for now
+
     /** Indicates if the widget is hidden or not */
     bool get_hidden() const { return _hidden; }
 
@@ -66,6 +73,7 @@ public:
     int get_indent() const { return _indent; }
 
 
+
     /** Recommended margin of boxes containing multiple Widgets (in px) */
     const static int GUI_BOX_MARGIN = 10;
     /** Recommended spacing between multiple Widgets packed into a box (in px) */