Optimize parameter string generation

Also rename overloaded string() functions, so it's clear what they
actually do, as it's not the same thing at all...

16 files changed, 65 insertions, 75 deletions

diff --git a/src/extension/prefdialog/parameter-bool.cpp b/src/extension/prefdialog/parameter-bool.cpp
index f3436374b..a21c38fb4 100644
--- a/src/extension/prefdialog/parameter-bool.cpp
+++ b/src/extension/prefdialog/parameter-bool.cpp
@@ -104,15 +104,12 @@ void ParamBoolCheckButton::on_toggle()
     return;
 }
 
-void ParamBool::string(std::string &string) const
+std::string ParamBool::value_to_string() const
 {
     if (_value) {
-        string += "true";
-    } else {
-        string += "false";
+        return "true";
     }
-
-    return;
+    return "false";
 }
 
 Gtk::Widget *ParamBool::get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal)
diff --git a/src/extension/prefdialog/parameter-bool.h b/src/extension/prefdialog/parameter-bool.h
index c5ffe2fd9..5b1b54e86 100644
--- a/src/extension/prefdialog/parameter-bool.h
+++ b/src/extension/prefdialog/parameter-bool.h
@@ -58,7 +58,7 @@ public:
      * Appends 'true' or 'false'.
      * @todo investigate. Returning a value that can then be appended would probably work better/safer.
      */
-    void string(std::string &string) const override;
+    std::string value_to_string() const override;
 
 private:
     /** Internal value. */
diff --git a/src/extension/prefdialog/parameter-color.cpp b/src/extension/prefdialog/parameter-color.cpp
index f99a2b54b..b55fcff5d 100644
--- a/src/extension/prefdialog/parameter-color.cpp
+++ b/src/extension/prefdialog/parameter-color.cpp
@@ -95,11 +95,11 @@ void ParamColor::_onColorChanged()
         _changeSignal->emit();
 }
 
-void ParamColor::string(std::string &string) const
+std::string ParamColor::value_to_string() const
 {
-    char str[16];
-    snprintf(str, 16, "%u", _color.value());
-    string += str;
+    char value_string[16];
+    snprintf(value_string, 16, "%u", _color.value());
+    return value_string;
 }
 
 };  /* namespace Extension */
diff --git a/src/extension/prefdialog/parameter-color.h b/src/extension/prefdialog/parameter-color.h
index 511320520..67175838f 100644
--- a/src/extension/prefdialog/parameter-color.h
+++ b/src/extension/prefdialog/parameter-color.h
@@ -44,7 +44,7 @@ public:
 
     Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal) override;
 
-    void string(std::string &string) const override;
+    std::string value_to_string() const override;
 
     sigc::signal<void> *_changeSignal;
 
diff --git a/src/extension/prefdialog/parameter-float.cpp b/src/extension/prefdialog/parameter-float.cpp
index f376de560..0f60f8665 100644
--- a/src/extension/prefdialog/parameter-float.cpp
+++ b/src/extension/prefdialog/parameter-float.cpp
@@ -107,12 +107,11 @@ float ParamFloat::set(float in, SPDocument * /*doc*/, Inkscape::XML::Node * /*no
     return _value;
 }
 
-void ParamFloat::string(std::string &string) const
+std::string ParamFloat::value_to_string() const
 {
-    char startstring[G_ASCII_DTOSTR_BUF_SIZE];
-    g_ascii_dtostr(startstring, G_ASCII_DTOSTR_BUF_SIZE, _value);
-    string += startstring;
-    return;
+    char value_string[G_ASCII_DTOSTR_BUF_SIZE];
+    g_ascii_dtostr(value_string, G_ASCII_DTOSTR_BUF_SIZE, _value);
+    return value_string;
 }
 
 /** A class to make an adjustment that uses Extension params. */
diff --git a/src/extension/prefdialog/parameter-float.h b/src/extension/prefdialog/parameter-float.h
index 93c38fb24..0b67233f1 100644
--- a/src/extension/prefdialog/parameter-float.h
+++ b/src/extension/prefdialog/parameter-float.h
@@ -46,7 +46,7 @@ public:
 
     Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal) override;
 
-    void string(std::string &string) const override;
+    std::string value_to_string() const override;
 
 private:
     /** Internal value. */
diff --git a/src/extension/prefdialog/parameter-int.cpp b/src/extension/prefdialog/parameter-int.cpp
index 53e858bdb..014872f12 100644
--- a/src/extension/prefdialog/parameter-int.cpp
+++ b/src/extension/prefdialog/parameter-int.cpp
@@ -176,11 +176,11 @@ ParamInt::get_widget (SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<v
     return dynamic_cast<Gtk::Widget *>(hbox);
 }
 
-void ParamInt::string(std::string &string) const
+std::string ParamInt::value_to_string() const
 {
-    char startstring[32];
-    sprintf(startstring, "%d", _value);
-    string += startstring;
+    char value_string[32];
+    snprintf(value_string, 32, "%d", _value);
+    return value_string;
 }
 
 }  // namespace Extension
diff --git a/src/extension/prefdialog/parameter-int.h b/src/extension/prefdialog/parameter-int.h
index 093bd3f03..13ca3ce82 100644
--- a/src/extension/prefdialog/parameter-int.h
+++ b/src/extension/prefdialog/parameter-int.h
@@ -44,7 +44,7 @@ public:
 
     Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal) override;
 
-    void string(std::string &string) const override;
+    std::string value_to_string() const override;
 
 private:
     /** Internal value. */
diff --git a/src/extension/prefdialog/parameter-notebook.cpp b/src/extension/prefdialog/parameter-notebook.cpp
index e84150aa2..50c21b3eb 100644
--- a/src/extension/prefdialog/parameter-notebook.cpp
+++ b/src/extension/prefdialog/parameter-notebook.cpp
@@ -24,13 +24,6 @@
 
 #include "xml/node.h"
 
-
-/**
- * The root directory in the preferences database for extension
- * related parameters.
- */
-#define PREF_DIR "extensions"
-
 namespace Inkscape {
 namespace Extension {
 
@@ -70,10 +63,10 @@ ParamNotebook::ParamNotebookPage::~ParamNotebookPage ()
     }
 }
 
-void ParamNotebook::ParamNotebookPage::string(std::list <std::string> &list) const
+void ParamNotebook::ParamNotebookPage::build_param_string_list(std::list <std::string> &list) const
 {
     for (auto parameter : parameters) {
-        parameter->string(list);
+        parameter->build_param_string_list(list);
     }
 }
 
@@ -209,23 +202,23 @@ const Glib::ustring& ParamNotebook::set(const int in, SPDocument * /*doc*/, Inks
     return _value;
 }
 
-void ParamNotebook::string(std::list <std::string> &list) const
+void ParamNotebook::build_param_string_list(std::list <std::string> &list) const
 {
-    std::string param_string;
-    param_string += "--";
-    param_string += _name;
-    param_string += "=";
-
-    param_string += "\"";
-    param_string += _value;  // the name of the current page
-    param_string += "\"";
-    list.insert(list.end(), param_string);
+    // call base-class method to add the parameter string for the notebook itself
+    InxParameter::build_param_string_list(list);
 
+    // iterate over notebook pages
     for (auto page : pages) {
-        page->string(list);
+        page->build_param_string_list(list);
     }
 }
 
+std::string ParamNotebook::value_to_string() const
+{
+    return _value;
+}
+
+
 /** A special category of Gtk::Notebook to handle notebook parameters. */
 class NotebookWidget : public Gtk::Notebook {
 private:
diff --git a/src/extension/prefdialog/parameter-notebook.h b/src/extension/prefdialog/parameter-notebook.h
index ea51162fe..90d032af3 100644
--- a/src/extension/prefdialog/parameter-notebook.h
+++ b/src/extension/prefdialog/parameter-notebook.h
@@ -54,7 +54,7 @@ private:
         ~ParamNotebookPage() override;
 
         Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal) override;
-        void string(std::list <std::string> &list) const override;
+        void build_param_string_list(std::list <std::string> &list) const override;
         char *get_text() { return _text; };
         InxParameter *get_param(const char *name) override;
     }; /* class ParamNotebookPage */
@@ -73,11 +73,9 @@ public:
      * A function to get the currentpage and the parameters in a string form.
      * @return A string with the 'value' and all the parameters on all pages as command line arguments.
      */
-    void string(std::list <std::string> &list) const override;
-
-    // Explicitly call superclass version to avoid method being hidden.
-    void string(std::string &string) const override {return InxParameter::string(string);}
+    void build_param_string_list(std::list <std::string> &list) const override;
 
+    std::string value_to_string() const override;
 
     InxParameter *get_param (const char *name) override;
 
diff --git a/src/extension/prefdialog/parameter-optiongroup.cpp b/src/extension/prefdialog/parameter-optiongroup.cpp
index 99775d8fc..78146568a 100644
--- a/src/extension/prefdialog/parameter-optiongroup.cpp
+++ b/src/extension/prefdialog/parameter-optiongroup.cpp
@@ -25,11 +25,6 @@
 #include "extension/extension.h"
 #include "preferences.h"
 
-/**
- * The root directory in the preferences database for extension
- * related parameters.
- */
-#define PREF_DIR "extensions"
 
 namespace Inkscape {
 namespace Extension {
@@ -139,9 +134,9 @@ bool ParamOptionGroup::contains(const Glib::ustring text, SPDocument const * /*d
     return false;
 }
 
-void ParamOptionGroup::string(std::string &string) const
+std::string ParamOptionGroup::value_to_string() const
 {
-    string += _value;
+    return _value;
 }
 
 /**
diff --git a/src/extension/prefdialog/parameter-optiongroup.h b/src/extension/prefdialog/parameter-optiongroup.h
index 17cd075e9..16607f2fb 100644
--- a/src/extension/prefdialog/parameter-optiongroup.h
+++ b/src/extension/prefdialog/parameter-optiongroup.h
@@ -47,7 +47,7 @@ public:
 
     Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal) override;
 
-    void string(std::string &string) const override;
+    std::string value_to_string() const override;
 
     Glib::ustring value_from_label(const Glib::ustring label);
 
diff --git a/src/extension/prefdialog/parameter-string.cpp b/src/extension/prefdialog/parameter-string.cpp
index 2836f1c99..05a8bca51 100644
--- a/src/extension/prefdialog/parameter-string.cpp
+++ b/src/extension/prefdialog/parameter-string.cpp
@@ -81,9 +81,9 @@ const Glib::ustring& ParamString::set(const Glib::ustring in, SPDocument * /*doc
     return _value;
 }
 
-void ParamString::string(std::string &string) const
+std::string ParamString::value_to_string() const
 {
-    string += _value;
+    return _value;
 }
 
 
diff --git a/src/extension/prefdialog/parameter-string.h b/src/extension/prefdialog/parameter-string.h
index e1abaa4e8..a8467f10f 100644
--- a/src/extension/prefdialog/parameter-string.h
+++ b/src/extension/prefdialog/parameter-string.h
@@ -28,7 +28,7 @@ public:
 
     Gtk::Widget *get_widget(SPDocument *doc, Inkscape::XML::Node *node, sigc::signal<void> *changeSignal) override;
 
-    void string(std::string &string) const override;
+    std::string value_to_string() const override;
 
     void setMaxLength(int maxLength) { _max_length = maxLength; }
     int getMaxLength() { return _max_length; }
diff --git a/src/extension/prefdialog/parameter.cpp b/src/extension/prefdialog/parameter.cpp
index 61cf8c662..3f8f30c60 100644
--- a/src/extension/prefdialog/parameter.cpp
+++ b/src/extension/prefdialog/parameter.cpp
@@ -57,7 +57,7 @@ public:
     }
 
     // Well, no, I don't have a value! That's why I should not be an InxParameter!
-    void string(std::string &/*string*/) const override {}
+    std::string value_to_string() const override { return ""; }
 };
 
 
@@ -264,25 +264,25 @@ char *InxParameter::pref_name() const
     return g_strdup_printf("%s.%s", _extension->get_id(), _name);
 }
 
-void InxParameter::string(std::string &/*string*/) const
+std::string InxParameter::value_to_string() const
 {
     // if we end up here we're missing a definition of ::string() in one of the subclasses
-    g_critical("InxParameter::string called from parameter '%s' in extension '%s'", _name, _extension->get_id());
+    g_critical("InxParameter::value_to_string called from parameter '%s' in extension '%s'", _name, _extension->get_id());
     g_assert_not_reached();
+    return "";
 }
 
-void InxParameter::string(std::list <std::string> &list) const
+void InxParameter::build_param_string_list(std::list <std::string> &list) const
 {
-    std::string value;
-    string(value);
-    if (!value.empty()) {
-        std::string final;
-        final += "--";
-        final += name();
-        final += "=";
-        final += value;
-
-        list.insert(list.end(), final);
+    std::string value_string = value_to_string();
+    if (!value_string.empty()) {
+        std::string parameter_string; // --param=value
+        parameter_string += "--";
+        parameter_string += name();
+        parameter_string += "=";
+        parameter_string += value_string;
+
+        list.insert(list.end(), parameter_string);
     }
 }
 
diff --git a/src/extension/prefdialog/parameter.h b/src/extension/prefdialog/parameter.h
index b5f662ebc..a5e8c0f71 100644
--- a/src/extension/prefdialog/parameter.h
+++ b/src/extension/prefdialog/parameter.h
@@ -107,7 +107,15 @@ public:
 
     const char *get_tooltip() const override { return _description; }
 
-    virtual void string(std::list <std::string> &list) const;
+    /**
+     * Build a list of parameter strings for the current parameter and it's children (if it has any)
+     *
+     * The individual parameter strings have the form "--param=value", so they can be passed to
+     * script extensions as-is
+     *
+     * @param list Reference to a list of strings that will be appended with the new parameter strings
+     */
+    virtual void build_param_string_list(std::list <std::string> &list) const;
 
     /**
      * Gets the current value of the parameter in a string form.
@@ -118,7 +126,7 @@ public:
      *           Unfortunately it seems we can't make this a pure virtual function,
      *           as InxParameter is not supposed to be abstract.
      */
-    virtual void string(std::string &string) const;
+    virtual std::string value_to_string() const;
 
     /** All the code in Notebook::get_param to get the notebook content. */
     virtual InxParameter *get_param(char const *name);