fix round-tripping of text runs with formatting

include/xlnt/cell/text_run.hpp

@@ -25,6 +25,7 @@
 #include <string>
 
 #include <xlnt/xlnt_config.hpp> // for XLNT_CLASS, XLNT_FUNCTION
+#include <xlnt/utils/optional.hpp>
 
 namespace xlnt {
 
@@ -38,21 +39,35 @@ public:
 
 	std::string get_string() const;
 	void set_string(const std::string &string);
-    
+
+    bool has_size() const;
     std::size_t get_size() const;
+    void set_size(std::size_t size);
+    
+    bool has_color() const;
     std::string get_color() const;
+    void set_color(const std::string &color);
+    
+    bool has_font() const;
     std::string get_font() const;
+    void set_font(const std::string &font);
+    
+    bool has_family() const;
     std::size_t get_family() const;
+    void set_family(std::size_t family);
+    
+    bool has_scheme() const;
     std::string get_scheme() const;
+    void set_scheme(const std::string &scheme);
 
 private:
 	std::string string_;
-    bool has_formatting_ = false;
+    
-    std::size_t size_;
+    std::experimental::optional<std::size_t> size_;
-    std::string color_;
+    std::experimental::optional<std::string> color_;
-    std::string font_;
+    std::experimental::optional<std::string> font_;
-    std::size_t family_;
+    std::experimental::optional<std::size_t> family_;
-    std::string scheme_;
+    std::experimental::optional<std::string> scheme_;
 };
 
 } // namespace xlnt

source/cell/text.cpp

@@ -71,11 +71,40 @@ bool text::operator==(const text &rhs) const
         
         if (runs_[i].has_formatting())
         {
-            if (runs_[i].get_color() != rhs.runs_[i].get_color()) return false;
+            if (runs_[i].has_color() == rhs.runs_[i].has_color()
-            if (runs_[i].get_family() != rhs.runs_[i].get_family()) return false;
+                && runs_[i].has_color()
-            if (runs_[i].get_font() != rhs.runs_[i].get_font()) return false;
+                && runs_[i].get_color() != rhs.runs_[i].get_color())
-            if (runs_[i].get_scheme() != rhs.runs_[i].get_scheme()) return false;
+            {
-            if (runs_[i].get_size() != rhs.runs_[i].get_size()) return false;
+                return false;
+            }
+            
+            if (runs_[i].has_family() == rhs.runs_[i].has_family()
+                && runs_[i].has_family()
+                && runs_[i].get_family() != rhs.runs_[i].get_family())
+            {
+                return false;
+            }
+            
+            if (runs_[i].has_font() == rhs.runs_[i].has_font()
+                && runs_[i].has_font()
+                && runs_[i].get_font() != rhs.runs_[i].get_font())
+            {
+                return false;
+            }
+            
+            if (runs_[i].has_scheme() == rhs.runs_[i].has_scheme()
+                && runs_[i].has_scheme()
+                && runs_[i].get_scheme() != rhs.runs_[i].get_scheme())
+            {
+                return false;
+            }
+            
+            if (runs_[i].has_size() == rhs.runs_[i].has_size()
+                && runs_[i].has_size()
+                && runs_[i].get_size() != rhs.runs_[i].get_size())
+            {
+                return false;
+            }
         }
     }
     

source/cell/text_run.cpp

@@ -45,32 +45,82 @@ void text_run::set_string(const std::string &string)
 
 bool text_run::has_formatting() const
 {
-    return has_formatting_;
+    return has_size() || has_color() || has_font() || has_family() || has_scheme();
+}
+
+bool text_run::has_size() const
+{
+    return (bool)size_;
 }
 
 std::size_t text_run::get_size() const
 {
-    return size_;
+    return *size_;
+}
+
+void text_run::set_size(std::size_t size)
+{
+    size_ = size;
+}
+
+bool text_run::has_color() const
+{
+    return (bool)color_;
 }
 
 std::string text_run::get_color() const
 {
-    return color_;
+    return *color_;
+}
+
+void text_run::set_color(const std::string &color)
+{
+    color_ = color;
+}
+
+bool text_run::has_font() const
+{
+    return (bool)font_;
 }
 
 std::string text_run::get_font() const
 {
-    return font_;
+    return *font_;
+}
+
+void text_run::set_font(const std::string &font)
+{
+    font_ = font;
+}
+
+bool text_run::has_family() const
+{
+    return (bool)family_;
 }
 
 std::size_t text_run::get_family() const
 {
-    return family_;
+    return *family_;
+}
+
+void text_run::set_family(std::size_t family)
+{
+    family_ = family;
+}
+
+bool text_run::has_scheme() const
+{
+    return (bool)scheme_;
 }
 
 std::string text_run::get_scheme() const
 {
-    return scheme_;
+    return *scheme_;
+}
+
+void text_run::set_scheme(const std::string &scheme)
+{
+    scheme_ = scheme;
 }
 
 } // namespace xlnt

source/serialization/shared_strings_serializer.cpp

@@ -37,6 +37,7 @@ xml_document shared_strings_serializer::write_shared_strings(const std::vector<t
 
     xml.add_namespace("", "http://schemas.openxmlformats.org/spreadsheetml/2006/main");
 
+    root_node.add_attribute("count", std::to_string(strings.size()));
     root_node.add_attribute("uniqueCount", std::to_string(strings.size()));
 
     for (const auto &string : strings)
@@ -47,24 +48,44 @@ xml_document shared_strings_serializer::write_shared_strings(const std::vector<t
         }
         else
         {
+            auto string_item_node = root_node.add_child("si");
+            
             for (const auto &run : string.get_runs())
             {
-                auto string_item_node = root_node.add_child("si");
                 auto rich_text_run_node = string_item_node.add_child("r");
                 
-                auto text_node = rich_text_run_node.add_child("t");
-                text_node.set_text(run.get_string());
-                
                 if (run.has_formatting())
                 {
                     auto run_properties_node = rich_text_run_node.add_child("rPr");
                     
-                    run_properties_node.add_child("sz").add_attribute("val", std::to_string(run.get_size()));
+                    if (run.has_size())
-                    run_properties_node.add_child("color").add_attribute("rgb", run.get_color());
+                    {
-                    run_properties_node.add_child("rFont").add_attribute("val", run.get_font());
+                        run_properties_node.add_child("sz").add_attribute("val", std::to_string(run.get_size()));
-                    run_properties_node.add_child("family").add_attribute("val", std::to_string(run.get_family()));
+                    }
-                    run_properties_node.add_child("scheme").add_attribute("val", run.get_scheme());
+                    
+                    if (run.has_color())
+                    {
+                        run_properties_node.add_child("color").add_attribute("rgb", run.get_color());
+                    }
+                    
+                    if (run.has_font())
+                    {
+                        run_properties_node.add_child("rFont").add_attribute("val", run.get_font());
+                    }
+                    
+                    if (run.has_family())
+                    {
+                        run_properties_node.add_child("family").add_attribute("val", std::to_string(run.get_family()));
+                    }
+                    
+                    if (run.has_scheme())
+                    {
+                        run_properties_node.add_child("scheme").add_attribute("val", run.get_scheme());
+                    }
                 }
+                
+                auto text_node = rich_text_run_node.add_child("t");
+                text_node.set_text(run.get_string());
             }
         }
     }
@@ -106,7 +127,39 @@ bool shared_strings_serializer::read_shared_strings(const xml_document &xml, std
                 if (rich_text_run_node.get_name() == "r" && rich_text_run_node.has_child("t"))
                 {
                     text_run run;
+                    
                     run.set_string(rich_text_run_node.get_child("t").get_text());
+                    
+                    if (rich_text_run_node.has_child("rPr"))
+                    {
+                        auto run_properties_node = rich_text_run_node.get_child("rPr");
+                        
+                        if (run_properties_node.has_child("sz"))
+                        {
+                            run.set_size(std::stoull(run_properties_node.get_child("sz").get_attribute("val")));
+                        }
+                        
+                        if (run_properties_node.has_child("rFont"))
+                        {
+                            run.set_font(run_properties_node.get_child("rFont").get_attribute("val"));
+                        }
+                        
+                        if (run_properties_node.has_child("color"))
+                        {
+                            run.set_color(run_properties_node.get_child("color").get_attribute("rgb"));
+                        }
+                        
+                        if (run_properties_node.has_child("family"))
+                        {
+                            run.set_family(std::stoull(run_properties_node.get_child("family").get_attribute("val")));
+                        }
+                        
+                        if (run_properties_node.has_child("scheme"))
+                        {
+                            run.set_scheme(run_properties_node.get_child("scheme").get_attribute("val"));
+                        }
+                    }
+                    
                     t.add_run(run);
                 }
             }

source/serialization/style_serializer.cpp

@@ -468,7 +468,7 @@ bool style_serializer::read_styles(const xlnt::xml_node &styles_node, const xlnt
     if (styles_.empty())
     {
         style ns;
-        ns.set_name("Standard");
+        ns.set_name("Normal");
         ns.set_hidden(false);
         ns.set_builtin_id(0);
         styles_.push_back(ns);
@@ -806,6 +806,14 @@ void style_serializer::initialize_vectors()
         {
             number_formats_.push_back(current_format.get_number_format());
         }
+        
+        auto existing_style = std::find_if(styles_.begin(), styles_.end(),
+            [&current_format](const style &s) { return s.get_name() == current_format.get_style_name(); });
+
+        if (existing_style == styles_.end())
+        {
+            styles_.push_back(workbook_.get_style(current_format.get_style_name()));
+        }
     }
     
     if (fonts_.empty())
@@ -1237,7 +1245,16 @@ bool style_serializer::write_formats(xml_node &formats_node) const
         
         if(current_format.has_style())
         {
-            xf_node.add_attribute("xfId", "0");
+            for (std::size_t i = 0; i < styles_.size(); i++)
+            {
+                const auto &current_style = styles_.at(i);
+                
+                if (current_style.get_name() == current_format.get_style_name())
+                {
+                    xf_node.add_attribute("xfId", std::to_string(i));
+                    break;
+                }
+            }
         }
     }
 

source/serialization/tests/test_read.hpp

@@ -14,6 +14,9 @@
 
 #include <helpers/path_helper.hpp>
 
+#include <xlnt/cell/text.hpp>
+#include <xlnt/cell/text_run.hpp>
+
 class test_read : public CxxTest::TestSuite
 {
 public:

source/styles/format.cpp

@@ -32,10 +32,16 @@
 namespace xlnt {
 
 format::format()
+    : formattable(),
+      parent_(nullptr),
+      named_style_name_("Normal")
 {
 }
 
-format::format(const format &other) : formattable(other)
+format::format(const format &other)
+    : formattable(other),
+      parent_(other.parent_),
+      named_style_name_(other.named_style_name_)
 {
 }
 
@@ -76,4 +82,9 @@ bool format::has_style() const
     return !named_style_name_.empty();
 }
 
+std::string format::get_style_name() const
+{
+    return named_style_name_;
+}
+
 } // namespace xlnt

source/workbook/workbook.cpp

@@ -749,4 +749,16 @@ const std::vector<format> &workbook::get_formats() const
     return d_->formats_;
 }
 
+style &workbook::get_style(const std::string &name)
+{
+    return *std::find_if(d_->styles_.begin(), d_->styles_.end(),
+        [&name](const style &s) { return s.get_name() == name; });
+}
+
+const style &workbook::get_style(const std::string &name) const
+{
+    return *std::find_if(d_->styles_.begin(), d_->styles_.end(),
+        [&name](const style &s) { return s.get_name() == name; });
+}
+
 } // namespace xlnt