Finished porting PlaceholderParser to XS

lib/Slic3r.pm

@@ -48,7 +48,6 @@ use Slic3r::GCode;
 use Slic3r::GCode::ArcFitting;
 use Slic3r::GCode::CoolingBuffer;
 use Slic3r::GCode::MotionPlanner;
-use Slic3r::GCode::PlaceholderParser;
 use Slic3r::GCode::PressureRegulator;
 use Slic3r::GCode::Reader;
 use Slic3r::GCode::SpiralVase;

lib/Slic3r/GCode.pm

@@ -482,10 +482,10 @@ sub set_extruder {
     
     # append custom toolchange G-code
     if (defined $self->writer->extruder && $self->config->toolchange_gcode) {
-        $gcode .= sprintf "%s\n", $self->placeholder_parser->process($self->config->toolchange_gcode, {
-            previous_extruder   => $self->writer->extruder->id,
-            next_extruder       => $extruder_id,
-        });
+        my $pp = $self->placeholder_parser->clone;
+        $pp->set('previous_extruder' => $self->writer->extruder->id);
+        $pp->set('next_extruder'     => $extruder_id);
+        $gcode .= sprintf "%s\n", $pp->process($self->config->toolchange_gcode);
     }
     
     # if ooze prevention is enabled, park current extruder in the nearest

lib/Slic3r/GCode/PlaceholderParser.pm

@@ -1,28 +0,0 @@
-package Slic3r::GCode::PlaceholderParser;
-use strict;
-use warnings;
-
-sub process {
-    my ($self, $string, $extra) = @_;
-    
-    # extra variables have priority over the stored ones
-    if ($extra) {
-        my $regex = join '|', keys %$extra;
-        $string =~ s/\[($regex)\]/$extra->{$1}/eg;
-    }
-    {
-        my $regex = join '|', @{$self->_single_keys};
-        $string =~ s/\[($regex)\]/$self->_single_get("$1")/eg;
-    }
-    {
-        my $regex = join '|', @{$self->_multiple_keys};
-        $string =~ s/\[($regex)\]/$self->_multiple_get("$1")/egx;
-        
-        # unhandled indices are populated using the first value
-        $string =~ s/\[($regex)_\d+\]/$self->_multiple_get("$1")/egx;
-    }
-    
-    return $string;
-}
-
-1;

lib/Slic3r/Print.pm

@@ -14,16 +14,6 @@ use Slic3r::Print::State ':steps';
 
 our $status_cb;
 
-sub new {
-    # TODO: port PlaceholderParser methods to C++, then its own constructor
-    # can call them and no need for this new() method at all
-    my ($class) = @_;
-    my $self = $class->_new;
-    $self->placeholder_parser->apply_env_variables;
-    $self->placeholder_parser->update_timestamp;
-    return $self;
-}
-
 sub set_status_cb {
     my ($class, $cb) = @_;
     $status_cb = $cb;

lib/Slic3r/Print/GCode.pm

@@ -378,15 +378,19 @@ sub process_layer {
     }
     
     # set new layer - this will change Z and force a retraction if retract_layer_change is enabled
-    $gcode .= $self->_gcodegen->placeholder_parser->process($self->print->config->before_layer_gcode, {
-        layer_num => $self->_gcodegen->layer_index + 1,
-        layer_z   => $layer->print_z,
-    }) . "\n" if $self->print->config->before_layer_gcode;
+    if ($self->print->config->before_layer_gcode) {
+        my $pp = $self->_gcodegen->placeholder_parser->clone;
+        $pp->set('layer_num' => $self->_gcodegen->layer_index + 1);
+        $pp->set('layer_z'   => $layer->print_z);
+        $gcode .= $pp->process($self->print->config->before_layer_gcode) . "\n";
+    }
     $gcode .= $self->_gcodegen->change_layer($layer);  # this will increase $self->_gcodegen->layer_index
-    $gcode .= $self->_gcodegen->placeholder_parser->process($self->print->config->layer_gcode, {
-        layer_num => $self->_gcodegen->layer_index,
-        layer_z   => $layer->print_z,
-    }) . "\n" if $self->print->config->layer_gcode;
+    if ($self->print->config->layer_gcode) {
+        my $pp = $self->_gcodegen->placeholder_parser->clone;
+        $pp->set('layer_num' => $self->_gcodegen->layer_index);
+        $pp->set('layer_z'   => $layer->print_z);
+        $gcode .= $pp->process($self->print->config->layer_gcode) . "\n";
+    }
     
     # extrude skirt along raft layers and normal object layers
     # (not along interlaced support material layers)

t/custom_gcode.t

@@ -47,7 +47,8 @@ use Slic3r::Test;
 {
     my $parser = Slic3r::GCode::PlaceholderParser->new;
     $parser->apply_config(my $config = Slic3r::Config->new_from_defaults);
-    is $parser->process('[temperature_[foo]]', { foo => '1' }),
+    $parser->set('foo' => '0');
+    is $parser->process('[temperature_[foo]]'),
         $config->temperature->[0],
         "nested config options";
 }

xs/src/libslic3r/PlaceholderParser.cpp

@@ -10,7 +10,7 @@ namespace Slic3r {
 
 PlaceholderParser::PlaceholderParser()
 {
-    this->_single["version"] = SLIC3R_VERSION;
+    this->set("version", SLIC3R_VERSION);
     this->apply_env_variables();
     this->update_timestamp();
 }
@@ -64,16 +64,17 @@ void PlaceholderParser::apply_config(DynamicPrintConfig &config)
         
         if (const ConfigOptionVectorBase* optv = dynamic_cast<const ConfigOptionVectorBase*>(opt)) {
             // set placeholders for options with multiple values
+            // TODO: treat [bed_shape] as single, not multiple
             this->set(key, optv->vserialize());
         } else if (const ConfigOptionPoint* optp = dynamic_cast<const ConfigOptionPoint*>(opt)) {
-            this->_single[key] = optp->serialize();
+            this->set(key, optp->serialize());
             
             Pointf val = *optp;
-            this->_multiple[key + "_X"] = val.x;
-            this->_multiple[key + "_Y"] = val.y;
+            this->set(key + "_X", val.x);
+            this->set(key + "_Y", val.y);
         } else {
             // set single-value placeholders
-            this->_single[key] = opt->serialize();
+            this->set(key, opt->serialize());
         }
     }
 }
@@ -109,18 +110,57 @@ PlaceholderParser::set(const std::string &key, int value)
 }
 
 void
-PlaceholderParser::set(const std::string &key, const std::vector<std::string> &values)
+PlaceholderParser::set(const std::string &key, std::vector<std::string> values)
 {
-    for (std::vector<std::string>::const_iterator v = values.begin(); v != values.end(); ++v) {
+    if (values.empty()) {
+        this->_multiple.erase(key);
+        this->_single.erase(key);
+    } else {
+        this->_multiple[key] = values;
+        this->_single[key] = values.front();
+    }
+}
+
+std::string
+PlaceholderParser::process(std::string str) const
+{
+    // replace single options, like [foo]
+    for (t_strstr_map::const_iterator it = this->_single.begin(); it != this->_single.end(); ++it) {
         std::stringstream ss;
-        ss << key << "_" << (v - values.begin());
-        
-        this->_multiple[ ss.str() ] = *v;
-        if (v == values.begin()) {
-            this->_multiple[key] = *v;
+        ss << '[' << it->first << ']';
+        this->find_and_replace(str, ss.str(), it->second);
+    }
+    
+    // replace multiple options like [foo_0] by looping until we have enough values
+    // or until a previous match was found (this handles non-existing indices reasonably
+    // without a regex)
+    for (t_strstrs_map::const_iterator it = this->_multiple.begin(); it != this->_multiple.end(); ++it) {
+        const std::vector<std::string> &values = it->second;
+        bool found = false;
+        for (size_t i = 0; (i < values.size()) || found; ++i) {
+            std::stringstream ss;
+            ss << '[' << it->first << '_' << i << ']';
+            if (i < values.size()) {
+                found = this->find_and_replace(str, ss.str(), values[i]);
+            } else {
+                found = this->find_and_replace(str, ss.str(), values.front());
+            }
         }
     }
-    this->_single.erase(key);
+    
+    return str;
+}
+
+bool
+PlaceholderParser::find_and_replace(std::string &source, std::string const &find, std::string const &replace) const
+{
+    bool found = false;
+    for (std::string::size_type i = 0; (i = source.find(find, i)) != std::string::npos; ) {
+        source.replace(i, find.length(), replace);
+        i += replace.length();
+        found = true;
+    }
+    return found;
 }
 
 #ifdef SLIC3RXS

xs/src/libslic3r/PlaceholderParser.hpp

@@ -11,19 +11,26 @@
 
 namespace Slic3r {
 
+typedef std::map<std::string, std::string> t_strstr_map;
+typedef std::map<std::string, std::vector<std::string> > t_strstrs_map;
+
 class PlaceholderParser
 {
     public:
-    std::map<std::string, std::string> _single;
-    std::map<std::string, std::string> _multiple;
-
+    t_strstr_map _single;
+    t_strstrs_map _multiple;
+    
     PlaceholderParser();
     void update_timestamp();
     void apply_config(DynamicPrintConfig &config);
     void apply_env_variables();
     void set(const std::string &key, const std::string &value);
     void set(const std::string &key, int value);
-    void set(const std::string &key, const std::vector<std::string> &values);
+    void set(const std::string &key, std::vector<std::string> values);
+    std::string process(std::string str) const;
+    
+    private:
+    bool find_and_replace(std::string &source, std::string const &find, std::string const &replace) const;
 };
 
 }

xs/xsp/PlaceholderParser.xsp

@@ -9,38 +9,14 @@
 %name{Slic3r::GCode::PlaceholderParser} class PlaceholderParser {
     PlaceholderParser();
     ~PlaceholderParser();
+    Clone<PlaceholderParser> clone()
+        %code{% RETVAL = THIS; %};
     
     void update_timestamp();
     void apply_env_variables();
     void apply_config(DynamicPrintConfig *config)
         %code%{ THIS->apply_config(*config); %};
     void set(std::string key, std::string value);
-    void set_multiple(std::string key, std::vector<std::string> values)
-        %code%{ THIS->set(key, values); %};
-
-    void _single_set(std::string k, std::string v)
-        %code%{ THIS->_single[k] = v; %};
-
-    std::string _single_get(std::string k)
-        %code%{ RETVAL = THIS->_single[k]; %};
-    std::string _multiple_get(std::string k)
-        %code%{ RETVAL = THIS->_multiple[k]; %};
-
-    std::vector<std::string> _single_keys()
-        %code{%
-            for (std::map<std::string, std::string>::iterator i = THIS->_single.begin();
-                i != THIS->_single.end(); ++i)
-            {
-                RETVAL.push_back(i->first);
-            }
-        %};
-
-    std::vector<std::string> _multiple_keys()
-        %code{%
-            for (std::map<std::string, std::string>::iterator i = THIS->_multiple.begin();
-                i != THIS->_multiple.end(); ++i)
-            {
-                RETVAL.push_back(i->first);
-            }
-        %};
+    %name{set_multiple} void set(std::string key, std::vector<std::string> values);
+    std::string process(std::string str) const;
 };

xs/xsp/Print.xsp

@@ -115,7 +115,7 @@ _constant()
 
 
 %name{Slic3r::Print} class Print {
-    %name{_new} Print();
+    Print();
     ~Print();
 
     Ref<PrintConfig> config()