implement more cell formatting

include/xlnt/styles/fill.hpp

@@ -86,9 +86,13 @@ public:
 
     void set_gradient_type(gradient_type t);
 
+    void set_foreground_color(const color &c);
+
     std::experimental::optional<color> &get_foreground_color();
 
     const std::experimental::optional<color> &get_foreground_color() const;
+    
+    void set_background_color(const color &c);
 
     std::experimental::optional<color> &get_background_color();
 

include/xlnt/workbook/workbook.hpp

@@ -37,6 +37,7 @@ namespace xlnt {
 class alignment;
 class app_properties;
 class border;
+class format;
 class color;
 class const_worksheet_iterator;
 class document_properties;
@@ -195,6 +196,8 @@ public:
 
     color add_indexed_color(const color &rgb_color);
     color get_indexed_color(const color &indexed_color) const;
+    
+    format &add_default_cell_format();
 
     const number_format &get_number_format(std::size_t style_id) const;
     std::size_t set_number_format(const number_format &format, std::size_t style_id);

source/cell/cell.cpp

@@ -607,12 +607,12 @@ const font &cell::get_font() const
 
 const fill &cell::get_fill() const
 {
-    return get_parent().get_parent().get_fill(d_->style_id_);
+    return get_parent().get_parent().get_fill(get_parent().get_parent().get_cell_format(d_->style_id_).get_fill_id());
 }
 
 const border &cell::get_border() const
 {
-    return get_parent().get_parent().get_border(d_->style_id_);
+    return get_parent().get_parent().get_border(get_parent().get_parent().get_cell_format(d_->style_id_).get_border_id());
 }
 
 const alignment &cell::get_alignment() const
@@ -627,12 +627,12 @@ const protection &cell::get_protection() const
 
 bool cell::pivot_button() const
 {
-    return get_parent().get_parent().get_pivot_button(d_->style_id_);
+    return d_->pivot_button_;
 }
 
 bool cell::quote_prefix() const
 {
-    return get_parent().get_parent().get_quote_prefix(d_->style_id_);
+    return d_->quote_prefix_;
 }
 
 void cell::clear_value()
@@ -747,6 +747,18 @@ XLNT_FUNCTION timedelta cell::get_value() const
     return timedelta::from_number(d_->value_numeric_);
 }
 
+void cell::set_border(const xlnt::border &border_)
+{
+    d_->has_style_ = true;
+    d_->style_id_ = get_parent().get_parent().set_border(border_, d_->style_id_);
+}
+
+void cell::set_fill(const xlnt::fill &fill_)
+{
+    d_->has_style_ = true;
+    d_->style_id_ = get_parent().get_parent().set_fill(fill_, d_->style_id_);
+}
+
 void cell::set_font(const font &font_)
 {
     d_->has_style_ = true;
@@ -759,6 +771,18 @@ void cell::set_number_format(const number_format &number_format_)
     d_->style_id_ = get_parent().get_parent().set_number_format(number_format_, d_->style_id_);
 }
 
+void cell::set_alignment(const xlnt::alignment &alignment_)
+{
+    d_->has_style_ = true;
+    d_->style_id_ = get_parent().get_parent().set_alignment(alignment_, d_->style_id_);
+}
+
+void cell::set_protection(const xlnt::protection &protection_)
+{
+    d_->has_style_ = true;
+    d_->style_id_ = get_parent().get_parent().set_protection(protection_, d_->style_id_);
+}
+
 template <>
 XLNT_FUNCTION std::string cell::get_value() const
 {
@@ -817,4 +841,14 @@ std::ostream &operator<<(std::ostream &stream, const xlnt::cell &cell)
     return stream << cell.to_string();
 }
 
+void cell::set_pivot_button(bool b)
+{
+    d_->pivot_button_ = b;
+}
+
+void cell::set_quote_prefix(bool b)
+{
+    d_->quote_prefix_ = b;
+}
+
 } // namespace xlnt

source/cell/tests/test_cell.hpp

@@ -92,12 +92,17 @@ public:
     
     void test_null()
     {
-        auto datatypes =
+        const auto datatypes =
         {
-            xlnt::cell::type::null
+            xlnt::cell::type::null,
+            xlnt::cell::type::boolean,
+            xlnt::cell::type::error,
+            xlnt::cell::type::formula,
+            xlnt::cell::type::numeric,
+            xlnt::cell::type::string
         };
         
-        for(auto datatype : datatypes)
+        for(const auto &datatype : datatypes)
         {
             auto ws = wb.create_sheet();
             auto cell = ws.get_cell(xlnt::cell_reference(1, 1));
@@ -135,7 +140,7 @@ public:
     
     void test_formula2()
     {
-        auto ws = wb.create_sheet();
+        auto ws = wb_guess_types.create_sheet();
         auto cell = ws.get_cell(xlnt::cell_reference(1, 1));
         
         cell.set_value("=if(A1<4;-1;1)");
@@ -246,7 +251,7 @@ public:
         TS_ASSERT(cell.get_value<bool>() == true);
     }
     
-    void test_illegal_chacters()
+    void test_illegal_characters()
     {
         auto ws = wb.create_sheet();
         auto cell = ws.get_cell(xlnt::cell_reference(1, 1));
@@ -267,6 +272,8 @@ public:
         cell.set_value(" Leading and trailing spaces are legal ");
     }
     
+    // void test_time_regex() {}
+    
     void test_timedelta()
     {
         auto ws = wb.create_sheet();
@@ -298,22 +305,6 @@ public:
         TS_ASSERT(cell.get_comment() == comm);
     }
     
-    void test_comment_count()
-    {
-        auto ws = wb.create_sheet();
-        auto cell = ws.get_cell(xlnt::cell_reference(1, 1));
-
-        TS_ASSERT(ws.get_comment_count() == 0);
-        xlnt::comment(cell, "text", "author");
-        TS_ASSERT(ws.get_comment_count() == 1);
-        xlnt::comment(cell, "text", "author");
-        TS_ASSERT(ws.get_comment_count() == 1);
-        cell.clear_comment();
-        TS_ASSERT(ws.get_comment_count() == 0);
-        cell.clear_comment();
-        TS_ASSERT(ws.get_comment_count() == 0);
-    }
-    
     void test_only_one_cell_per_comment()
     {
         auto ws = wb.create_sheet();
@@ -375,29 +366,29 @@ public:
         TS_ASSERT_EQUALS(cell.get_font(), font);
     }
     
-    void _test_fill()
+    void test_fill()
     {
         xlnt::fill f;
-        f.set_type(xlnt::fill::type::solid);
+        f.set_pattern_type(xlnt::fill::pattern_type::solid);
-//        f.set_foreground_color("FF0000");
+        f.set_foreground_color(xlnt::color(xlnt::color::type::rgb, "FF0000"));
         auto ws = wb.create_sheet();
-        ws.get_parent().add_fill(f);
-    
         xlnt::cell cell(ws, "A1");
+        cell.set_fill(f);
         TS_ASSERT(cell.get_fill() == f);
     }
     
-    void _test_border()
+    void test_border()
     {
         xlnt::border border;
         auto ws = wb.create_sheet();
-        ws.get_parent().add_border(border);
         
         auto cell = ws.get_cell(xlnt::cell_reference(1, 1));
+        cell.set_border(border);
+        
         TS_ASSERT(cell.get_border() == border);
     }
     
-    void _test_number_format()
+    void test_number_format()
     {
         auto ws = wb.create_sheet();
         ws.get_parent().add_number_format(xlnt::number_format("dd--hh--mm"));
@@ -407,31 +398,33 @@ public:
         TS_ASSERT(cell.get_number_format().get_format_string() == "dd--hh--mm");
     }
     
-    void _test_alignment()
+    void test_alignment()
     {
         xlnt::alignment align;
         align.set_wrap_text(true);
         
         auto ws = wb.create_sheet();
-        ws.get_parent().add_alignment(align);
     
         xlnt::cell cell(ws, "A1");
+        cell.set_alignment(align);
+        
         TS_ASSERT(cell.get_alignment() == align);
     }
     
-    void _test_protection()
+    void test_protection()
     {
         xlnt::protection prot;
         prot.set_locked(xlnt::protection::type::protected_);
         
         auto ws = wb.create_sheet();
-        ws.get_parent().add_protection(prot);
     
         xlnt::cell cell(ws, "A1");
+        cell.set_protection(prot);
+        
         TS_ASSERT(cell.get_protection() == prot);
     }
     
-    void _test_pivot_button()
+    void test_pivot_button()
     {
         auto ws = wb.create_sheet();
         
@@ -440,7 +433,7 @@ public:
         TS_ASSERT(cell.pivot_button());
     }
     
-    void _test_quote_prefix()
+    void test_quote_prefix()
     {
         auto ws = wb.create_sheet();
     
@@ -448,43 +441,4 @@ public:
         cell.set_quote_prefix(true);
         TS_ASSERT(cell.quote_prefix());
     }
-
-    void test_check_string()
-    {
-        xlnt::workbook utf8_wb(xlnt::encoding::utf8);
-        auto ws = utf8_wb.get_active_sheet();
-        auto cell = ws[xlnt::cell_reference("A1")];
-
-	std::vector<std::string> valid_utf8_strings = 
-        {
-            "a",
-            "\xc3\xa0",
-            "\xc3\xb1",
-	    "\xe2\x82\xa1",
-	    "\xf0\x90\x8c\xbc",
-	};
-
-	for(auto valid : valid_utf8_strings)
-        {
-	    TS_ASSERT_THROWS_NOTHING(cell.check_string(valid));
-	    TS_ASSERT_THROWS_NOTHING(cell.set_value(valid));
-        }
-
-	std::vector<std::string> invalid_utf8_strings = 
-        {
-            "\xc3\x28",
-	    "\xa0\xa1",
-	    "\xe2\x28\xa1",
-	    "\xe2\x82\x28",
-	    "\xf0\x28\x8c\xbc",
-	    "\xf0\x90\x28\xbc",
-	    "\xf0\x28\x8c\x28",
-        };
-
-	for(auto invalid : invalid_utf8_strings)
-        {
-	    TS_ASSERT_THROWS(cell.check_string(invalid), xlnt::unicode_decode_error);
-	    TS_ASSERT_THROWS(cell.set_value(invalid), xlnt::unicode_decode_error);
-        }
-    }
 };

source/detail/cell_impl.hpp

@@ -75,6 +75,9 @@ struct cell_impl
     std::size_t style_id_;
 
     std::unique_ptr<comment_impl> comment_;
+    
+    bool pivot_button_;
+    bool quote_prefix_;
 };
 
 } // namespace detail

source/serialization/style_serializer.cpp

@@ -1054,7 +1054,7 @@ xml_document style_serializer::write_stylesheet() const
     for (auto &format : formats)
     {
         auto xf_node = cell_xfs_node.add_child("xf");
-        xf_node.add_attribute("numFmtId", std::to_string(format.get_number_format().get_id()));
+        xf_node.add_attribute("numFmtId", std::to_string(format.get_number_format_id()));
         xf_node.add_attribute("fontId", std::to_string(format.get_font_id()));
 
         if (format.fill_apply_)

source/styles/fill.cpp

@@ -123,6 +123,11 @@ void fill::set_gradient_type(gradient_type t)
     gradient_type_ = t;
 }
 
+void fill::set_foreground_color(const color &c)
+{
+    foreground_color_ = c;
+}
+
 std::experimental::optional<color> &fill::get_foreground_color()
 {
     return foreground_color_;

source/workbook/workbook.cpp

@@ -601,6 +601,16 @@ void workbook::add_number_format(const number_format &number_format_)
     }
 }
 
+void workbook::add_protection(const xlnt::protection &p)
+{
+
+}
+
+void workbook::add_alignment(const xlnt::alignment &a)
+{
+
+}
+
 void workbook::set_code_name(const std::string & /*code_name*/)
 {
 }
@@ -701,6 +711,50 @@ const font &workbook::get_font(std::size_t font_id) const
     return d_->fonts_[font_id];
 }
 
+format &workbook::add_default_cell_format()
+{
+    format new_format;
+
+    new_format.id_ = 0;
+    new_format.border_id_ = 0;
+    new_format.fill_id_ = 0;
+    new_format.font_id_ = 0;
+    new_format.number_format_id_ = 0;
+
+    if (d_->borders_.empty())
+    {
+        d_->borders_.push_back(new_format.get_border());
+    }
+
+    if (d_->fills_.empty())
+    {
+        d_->fills_.push_back(new_format.get_fill());
+    }
+
+    if (d_->fonts_.empty())
+    {
+        d_->fonts_.push_back(new_format.get_font());
+    }
+
+    if (d_->number_formats_.empty())
+    {
+        d_->number_formats_.push_back(new_format.get_number_format());
+    }
+
+    d_->cell_formats_.push_back(new_format);
+    
+    return d_->cell_formats_[0];
+}
+
+// find font in font list
+// if not found, add to font list and make font_id equal to list length - 1
+// otherwise, font_id is the index of the matching font in font list
+// if there are no cell formats defined yet, create a new format with components set to default and font as parameter font, return 0
+// if the font is identical to the font for the cell's format, just return the provided cell format id
+// make a new cell format based on the existing cell format
+// set font on this style to the parameter font
+// see if this modified format already exists in the font list, if so return its id
+// otherwise, add the new cell format to the list of formats and return its id
 std::size_t workbook::set_font(const font &font_, std::size_t format_id)
 {
     auto match = std::find(d_->fonts_.begin(), d_->fonts_.end(), font_);
@@ -718,52 +772,31 @@ std::size_t workbook::set_font(const font &font_, std::size_t format_id)
 
     if (d_->cell_formats_.empty())
     {
-        format new_format;
+        auto &added = add_default_cell_format();
-
+        
-        new_format.id_ = 0;
+        added.font_id_ = font_id;
-        new_format.border_id_ = 0;
+        added.font_ = font_;
-        new_format.fill_id_ = 0;
+        added.font_apply_ = true;
-        new_format.font_id_ = font_id;
+        
-        new_format.font_apply_ = true;
-        new_format.number_format_id_ = 0;
-
-        if (d_->borders_.empty())
-        {
-            d_->borders_.push_back(new_format.get_border());
-        }
-
-        if (d_->fills_.empty())
-        {
-            d_->fills_.push_back(new_format.get_fill());
-        }
-
-        if (d_->number_formats_.empty())
-        {
-            d_->number_formats_.push_back(new_format.get_number_format());
-        }
-
-        d_->cell_formats_.push_back(new_format);
-
         return 0;
     }
 
-    // If the style is unchanged, just return it.
+    auto &existing_format = d_->cell_formats_[format_id];
-    auto &existing_style = d_->cell_formats_[format_id];
+    existing_format.font_apply_ = true;
-    existing_style.font_apply_ = true;
 
-    if (font_id == existing_style.font_id_)
+    // If the style is unchanged, just return it.
+    if (font_id == existing_format.font_id_)
     {
-        // no change
         return format_id;
     }
 
-    // Make a new style with this format.
+    // Make a new format based on existing format.
-    auto new_format = existing_style;
+    auto new_format = existing_format;
 
     new_format.font_id_ = font_id;
     new_format.font_ = font_;
 
-    // Check if the new style is already applied to a different cell. If so, reuse it.
+    // Check if the new modified style is already applied to a different cell. If so, reuse it.
     auto format_match = std::find(d_->cell_formats_.begin(), d_->cell_formats_.end(), new_format);
 
     if (format_match != d_->cell_formats_.end())
@@ -778,14 +811,70 @@ std::size_t workbook::set_font(const font &font_, std::size_t format_id)
     return new_format.id_;
 }
 
+const format &workbook::get_cell_format(std::size_t format_id) const
+{
+    return d_->cell_formats_.at(format_id);
+}
+
 const fill &workbook::get_fill(std::size_t fill_id) const
 {
     return d_->fills_[fill_id];
 }
 
-std::size_t workbook::set_fill(const fill & /*fill_*/, std::size_t format_id)
+std::size_t workbook::set_fill(const fill & fill_, std::size_t format_id)
 {
-    return format_id;
+    auto match = std::find(d_->fills_.begin(), d_->fills_.end(), fill_);
+    std::size_t fill_id = 0;
+
+    if (match == d_->fills_.end())
+    {
+        d_->fills_.push_back(fill_);
+        fill_id = d_->fills_.size() - 1;
+    }
+    else
+    {
+        fill_id = match - d_->fills_.begin();
+    }
+
+    if (d_->cell_formats_.empty())
+    {
+        auto &added = add_default_cell_format();
+        
+        added.fill_id_ = fill_id;
+        added.fill_ = fill_;
+        added.fill_apply_ = true;
+        
+        return 0;
+    }
+
+    auto &existing_format = d_->cell_formats_[format_id];
+    existing_format.fill_apply_ = true;
+
+    // If the style is unchanged, just return it.
+    if (fill_id == existing_format.fill_id_)
+    {
+        return format_id;
+    }
+
+    // Make a new format based on existing format.
+    auto new_format = existing_format;
+
+    new_format.fill_id_ = fill_id;
+    new_format.fill_ = fill_;
+
+    // Check if the new modified style is already applied to a different cell. If so, reuse it.
+    auto format_match = std::find(d_->cell_formats_.begin(), d_->cell_formats_.end(), new_format);
+
+    if (format_match != d_->cell_formats_.end())
+    {
+        return format_match->get_id();
+    }
+
+    // No match found, so add it.
+    new_format.id_ = d_->cell_formats_.size();
+    d_->cell_formats_.push_back(new_format);
+
+    return new_format.id_;
 }
 
 const border &workbook::get_border(std::size_t border_id) const
@@ -793,9 +882,60 @@ const border &workbook::get_border(std::size_t border_id) const
     return d_->borders_[border_id];
 }
 
-std::size_t workbook::set_border(const border & /*border_*/, std::size_t format_id)
+std::size_t workbook::set_border(const border & border_, std::size_t format_id)
 {
-    return format_id;
+    auto match = std::find(d_->borders_.begin(), d_->borders_.end(), border_);
+    std::size_t border_id = 0;
+
+    if (match == d_->borders_.end())
+    {
+        d_->borders_.push_back(border_);
+        border_id = d_->borders_.size() - 1;
+    }
+    else
+    {
+        border_id = match - d_->borders_.begin();
+    }
+
+    if (d_->cell_formats_.empty())
+    {
+        auto &added = add_default_cell_format();
+        
+        added.border_id_ = border_id;
+        added.border_ = border_;
+        added.border_apply_ = true;
+        
+        return 0;
+    }
+
+    auto &existing_format = d_->cell_formats_[format_id];
+    existing_format.border_apply_ = true;
+
+    // If the style is unchanged, just return it.
+    if (border_id == existing_format.border_id_)
+    {
+        return format_id;
+    }
+
+    // Make a new format based on existing format.
+    auto new_format = existing_format;
+
+    new_format.border_id_ = border_id;
+    new_format.border_ = border_;
+
+    // Check if the new modified style is already applied to a different cell. If so, reuse it.
+    auto format_match = std::find(d_->cell_formats_.begin(), d_->cell_formats_.end(), new_format);
+
+    if (format_match != d_->cell_formats_.end())
+    {
+        return format_match->get_id();
+    }
+
+    // No match found, so add it.
+    new_format.id_ = d_->cell_formats_.size();
+    d_->cell_formats_.push_back(new_format);
+
+    return new_format.id_;
 }
 
 const alignment &workbook::get_alignment(std::size_t format_id) const
@@ -803,9 +943,46 @@ const alignment &workbook::get_alignment(std::size_t format_id) const
     return d_->cell_formats_[format_id].alignment_;
 }
 
-std::size_t workbook::set_alignment(const alignment & /*alignment_*/, std::size_t format_id)
+// differs from border, fill, font, number_format because alignment is a member of format
+std::size_t workbook::set_alignment(const alignment & alignment_, std::size_t format_id)
 {
-    return format_id;
+    if (d_->cell_formats_.empty())
+    {
+        auto &added = add_default_cell_format();
+        
+        added.alignment_ = alignment_;
+        added.alignment_apply_ = true;
+        
+        return 0;
+    }
+
+    auto &existing_format = d_->cell_formats_[format_id];
+    existing_format.alignment_apply_ = true;
+
+    // If the style is unchanged, just return it.
+    if (alignment_ == existing_format.alignment_)
+    {
+        return format_id;
+    }
+
+    // Make a new format based on existing format.
+    auto new_format = existing_format;
+
+    new_format.alignment_ = alignment_;
+
+    // Check if the new modified style is already applied to a different cell. If so, reuse it.
+    auto format_match = std::find(d_->cell_formats_.begin(), d_->cell_formats_.end(), new_format);
+
+    if (format_match != d_->cell_formats_.end())
+    {
+        return format_match->get_id();
+    }
+
+    // No match found, so add it.
+    new_format.id_ = d_->cell_formats_.size();
+    d_->cell_formats_.push_back(new_format);
+
+    return new_format.id_;
 }
 
 const protection &workbook::get_protection(std::size_t format_id) const
@@ -813,9 +990,46 @@ const protection &workbook::get_protection(std::size_t format_id) const
     return d_->cell_formats_[format_id].protection_;
 }
 
-std::size_t workbook::set_protection(const protection & /*protection_*/, std::size_t format_id)
+// differs from border, fill, font, number_format because protection is a member of format
+std::size_t workbook::set_protection(const protection & protection_, std::size_t format_id)
 {
-    return format_id;
+    if (d_->cell_formats_.empty())
+    {
+        auto &added = add_default_cell_format();
+        
+        added.protection_ = protection_;
+        added.protection_apply_ = true;
+        
+        return 0;
+    }
+
+    auto &existing_format = d_->cell_formats_[format_id];
+    existing_format.protection_apply_ = true;
+
+    // If the style is unchanged, just return it.
+    if (protection_ == existing_format.protection_)
+    {
+        return format_id;
+    }
+
+    // Make a new format based on existing format.
+    auto new_format = existing_format;
+
+    new_format.protection_ = protection_;
+
+    // Check if the new modified style is already applied to a different cell. If so, reuse it.
+    auto format_match = std::find(d_->cell_formats_.begin(), d_->cell_formats_.end(), new_format);
+
+    if (format_match != d_->cell_formats_.end())
+    {
+        return format_match->get_id();
+    }
+
+    // No match found, so add it.
+    new_format.id_ = d_->cell_formats_.size();
+    d_->cell_formats_.push_back(new_format);
+
+    return new_format.id_;
 }
 
 bool workbook::get_pivot_button(std::size_t format_id) const
@@ -829,75 +1043,54 @@ bool workbook::get_quote_prefix(std::size_t format_id) const
 }
 
 //TODO: this is terrible!
-std::size_t workbook::set_number_format(const xlnt::number_format &nf, std::size_t style_id)
+std::size_t workbook::set_number_format(const xlnt::number_format &number_format_, std::size_t format_id)
 {
-    auto match = std::find(d_->number_formats_.begin(), d_->number_formats_.end(), nf);
+    auto match = std::find(d_->number_formats_.begin(), d_->number_formats_.end(), number_format_);
-    std::size_t format_id = 0;
+    std::size_t number_format_id = 0;
 
     if (match == d_->number_formats_.end())
     {
-        d_->number_formats_.push_back(nf);
+        d_->number_formats_.push_back(number_format_);
 
-        if (!nf.has_id())
+        if (!number_format_.has_id())
         {
             d_->number_formats_.back().set_id(d_->next_custom_format_id_++);
         }
 
-        format_id = d_->number_formats_.back().get_id();
+        number_format_id = d_->number_formats_.back().get_id();
     }
     else
     {
-        format_id = match->get_id();
+        number_format_id = match->get_id();
     }
 
     if (d_->cell_formats_.empty())
     {
-        format new_format;
+        auto &added = add_default_cell_format();
-
+        
-        new_format.id_ = 0;
+        added.number_format_id_ = number_format_id;
-        new_format.border_id_ = 0;
+        added.number_format_ = number_format_;
-        new_format.fill_id_ = 0;
+        added.number_format_apply_ = true;
-        new_format.font_id_ = 0;
+        
-        new_format.number_format_id_ = format_id;
-        new_format.number_format_apply_ = true;
-
-        if (d_->borders_.empty())
-        {
-            d_->borders_.push_back(new_format.get_border());
-        }
-
-        if (d_->fills_.empty())
-        {
-            d_->fills_.push_back(new_format.get_fill());
-        }
-
-        if (d_->fonts_.empty())
-        {
-            d_->fonts_.push_back(new_format.get_font());
-        }
-
-        d_->cell_formats_.push_back(new_format);
-
         return 0;
     }
 
-    // If the style is unchanged, just return it.
+    auto &existing_format = d_->cell_formats_[format_id];
-    auto existing_style = d_->cell_formats_[format_id];
+    existing_format.number_format_apply_ = true;
-    existing_style.number_format_apply_ = true;
 
-    if (format_id == existing_style.number_format_id_)
+    // If the style is unchanged, just return it.
+    if (number_format_id == existing_format.number_format_id_)
     {
-        // no change
+        return format_id;
-        return style_id;
     }
 
-    // Make a new style with this format.
+    // Make a new format based on existing format.
-    auto new_format = existing_style;
+    auto new_format = existing_format;
 
-    new_format.number_format_id_ = format_id;
+    new_format.number_format_id_ = number_format_id;
-    new_format.number_format_ = nf;
+    new_format.number_format_ = number_format_;
 
-    // Check if the new style is already applied to a different cell. If so, reuse it.
+    // Check if the new modified style is already applied to a different cell. If so, reuse it.
     auto format_match = std::find(d_->cell_formats_.begin(), d_->cell_formats_.end(), new_format);
 
     if (format_match != d_->cell_formats_.end())