improve manifest

include/xlnt/packaging/manifest.hpp

@@ -39,6 +39,8 @@ namespace xlnt {
 class XLNT_CLASS manifest
 {
 public:
+    void clear();
+
     bool has_default_type(const std::string &extension) const;
     std::string get_default_type(const std::string &extension) const;
     const std::unordered_map<std::string, default_type> &get_default_types() const;

include/xlnt/worksheet/worksheet.hpp

@@ -212,6 +212,7 @@ public:
 
     std::vector<std::string> get_formula_attributes() const;
 
+    sheet_state get_sheet_state() const;
     void set_sheet_state(sheet_state state);
 
     iterator begin();

source/cell/cell_reference.cpp

@@ -69,12 +69,10 @@ cell_reference::cell_reference(const std::string &column, row_t row)
 cell_reference::cell_reference(column_t column_index, row_t row)
     : column_(column_index), row_(row), absolute_row_(false), absolute_column_(false)
 {
-    if (row_ == 0 || column_ == 0)
-    {
-        throw value_error();
-    }
-
-    if (!(row_ <= constants::max_row()) || !(column_ <= constants::max_column()))
+    if (row_ == 0
+        || column_ == 0
+        || !(row_ <= constants::max_row())
+        || !(column_ <= constants::max_column()))
     {
         throw cell_coordinates_error(column_, row_);
     }
@@ -111,6 +109,12 @@ range_reference cell_reference::to_range() const
     return range_reference(column_, row_, column_, row_);
 }
 
+std::pair<std::string, row_t> cell_reference::split_reference(const std::string &reference_string)
+{
+    bool ignore1, ignore2;
+    return split_reference(reference_string, ignore1, ignore2);
+}
+
 std::pair<std::string, row_t> cell_reference::split_reference(const std::string &reference_string,
                                                               bool &absolute_column, bool &absolute_row)
 {

source/detail/excel_serializer.cpp

@@ -68,6 +68,8 @@ std::string::size_type find_string_in_string(const std::string &string, const st
 
 bool load_workbook(xlnt::zip_file &archive, bool guess_types, bool data_only, xlnt::workbook &wb, xlnt::detail::stylesheet &stylesheet)
 {
+    wb.clear();
+
     wb.set_guess_types(guess_types);
     wb.set_data_only(data_only);
 
@@ -86,8 +88,6 @@ bool load_workbook(xlnt::zip_file &archive, bool guess_types, bool data_only, xl
         throw xlnt::invalid_file_error("package is not an OOXML SpreadsheetML");
     }
 
-    wb.clear();
-    
     if(archive.has_file(xlnt::constants::part_core()))
     {
         xlnt::workbook_serializer workbook_serializer_(wb);

source/detail/manifest_serializer.cpp

@@ -73,12 +73,9 @@ void manifest_serializer::write_manifest(pugi::xml_document &xml) const
 
 std::string manifest_serializer::determine_document_type() const
 {
-    if (!manifest_.has_override_type(constants::part_workbook()))
+    for (auto current_override_type : manifest_.get_override_types())
     {
-        return "unsupported";
-    }
-
-    std::string type = manifest_.get_override_type(constants::part_workbook());
+        auto type = current_override_type.second.get_content_type();
 
         if (type == "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml")
         {
@@ -92,6 +89,7 @@ std::string manifest_serializer::determine_document_type() const
         {
             return "word";
         }
+    }
 
     return "unsupported";
 }

source/detail/workbook_serializer.cpp

@@ -290,6 +290,12 @@ void workbook_serializer::write_workbook(pugi::xml_document &xml) const
             auto sheet_node = sheets_node.append_child("sheet");
             sheet_node.append_attribute("name").set_value(ws.get_title().c_str());
             sheet_node.append_attribute("sheetId").set_value(std::to_string(ws.get_id()).c_str());
+            
+            if (ws.get_sheet_state() == xlnt::sheet_state::hidden)
+            {
+                sheet_node.append_attribute("state").set_value("hidden");
+            }
+            
             sheet_node.append_attribute("r:id").set_value(rel.get_id().c_str());
 
             if (ws.has_auto_filter())
@@ -317,7 +323,15 @@ void workbook_serializer::write_named_ranges(pugi::xml_node node) const
 {
     for (auto &named_range : workbook_.get_named_ranges())
     {
-        node.append_child(named_range.get_name().c_str());
+        auto defined_name_node = node.append_child("s:definedName");
+        defined_name_node.append_attribute("xmlns:s").set_value("http://schemas.openxmlformats.org/spreadsheetml/2006/main");
+        defined_name_node.append_attribute("name").set_value(named_range.get_name().c_str());
+        const auto &target = named_range.get_targets().front();
+        std::string target_string = "'" + target.first.get_title();
+        target_string.push_back('\'');
+        target_string.push_back('!');
+        target_string.append(target.second.to_string());
+        defined_name_node.text().set(target_string.c_str());
     }
 }
 

source/packaging/manifest.cpp

@@ -28,6 +28,12 @@
 
 namespace xlnt {
 
+void manifest::clear()
+{
+    default_types_.clear();
+    override_types_.clear();
+}
+
 bool manifest::has_default_type(const std::string &extension) const
 {
     return default_types_.find(extension) != default_types_.end();

source/workbook/tests/test_read.hpp

@@ -561,7 +561,8 @@ public:
         TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("2_not-empty.txt")), xlnt::invalid_file_error);
         TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("3_empty.zip")), xlnt::invalid_file_error);
         TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("4_not-package.zip")), xlnt::invalid_file_error);
-        TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("5_document.docx")), xlnt::invalid_file_error);
-        TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("6_presentation.pptx")), xlnt::invalid_file_error);
+        TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("5_visio.vsdx")), xlnt::invalid_file_error);
+        TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("6_document.docx")), xlnt::invalid_file_error);
+        TS_ASSERT_THROWS(wb.load(path_helper::get_data_directory("7_presentation.pptx")), xlnt::invalid_file_error);
     }
 };

source/workbook/tests/test_write_workbook.hpp

@@ -14,7 +14,7 @@ public:
     void test_write_auto_filter()
     {
         xlnt::workbook wb;
-        auto ws = wb.create_sheet();
+        auto ws = wb.get_active_sheet();
         ws.get_cell("F42").set_value("hello");
         ws.auto_filter("A1:F1");
     
@@ -22,14 +22,13 @@ public:
         pugi::xml_document xml;
         serializer.write_workbook(xml);
 
-        TS_SKIP("");
-        TS_ASSERT(xml_helper::compare_xml(path_helper::read_file("workbook_auto_filter.xml"), xml));
+        TS_ASSERT(xml_helper::compare_xml(path_helper::get_data_directory("/writer/expected/workbook_auto_filter.xml"), xml));
     }
     
     void test_write_hidden_worksheet()
     {
         xlnt::workbook wb;
-        auto ws = wb.create_sheet();
+        auto ws = wb.get_active_sheet();
         ws.set_sheet_state(xlnt::sheet_state::hidden);
         wb.create_sheet();
 
@@ -54,8 +53,8 @@ public:
         pugi::xml_document expected;
         expected.load(expected_string.c_str());
 
-        TS_SKIP("");
-        TS_ASSERT(xml_helper::compare_xml(expected, xml));
+        TS_ASSERT(xml_helper::compare_xml(expected.child("workbook").child("sheets"),
+            xml.child("workbook").child("sheets")));
     }
     
     void test_write_hidden_single_worksheet()
@@ -96,43 +95,43 @@ public:
     void test_write_workbook_rels()
     {
         xlnt::workbook wb;
+        auto ws = wb.get_active_sheet();
+        ws.get_cell("A1").set_value("string");
+
         xlnt::zip_file archive;
         xlnt::relationship_serializer serializer(archive);
         serializer.write_relationships(wb.get_relationships(), "xl/workbook.xml");
         pugi::xml_document observed;
         observed.load(archive.read("xl/_rels/workbook.xml.rels").c_str());
-        auto filename = "workbook.xml.rels";
 
-        TS_SKIP("");
-        TS_ASSERT(xml_helper::compare_xml(path_helper::read_file(filename), observed));
+        TS_ASSERT(xml_helper::compare_xml(path_helper::get_data_directory("/writer/expected/workbook.xml.rels"), observed));
     }
     
-    void test_write_workbook_()
+    void test_write_workbook_part()
     {
         xlnt::workbook wb;
         xlnt::workbook_serializer serializer(wb);
         pugi::xml_document xml;
         serializer.write_workbook(xml);
-        auto filename = path_helper::get_data_directory("/workbook.xml");
+        auto filename = path_helper::get_data_directory("/writer/expected/workbook.xml");
 
-        TS_SKIP("");
-        TS_ASSERT(xml_helper::compare_xml(path_helper::read_file(filename), xml));
+        TS_ASSERT(xml_helper::compare_xml(filename, xml));
     }
     
     void test_write_named_range()
     {
         xlnt::workbook wb;
-        auto ws = wb.create_sheet();
-        wb.create_named_range("test_range", ws, "A1:B5");
+        auto ws = wb.get_active_sheet();
+        wb.create_named_range("test_range", ws, "$A$1:$B$5");
         xlnt::workbook_serializer serializer(wb);
         pugi::xml_document xml;
-        serializer.write_named_ranges(xml.root());
+        auto root = xml.root().append_child("root");
+        serializer.write_named_ranges(root);
         std::string expected =
         "<root>"
             "<s:definedName xmlns:s=\"http://schemas.openxmlformats.org/spreadsheetml/2006/main\" name=\"test_range\">'Sheet'!$A$1:$B$5</s:definedName>"
         "</root>";
 
-        TS_SKIP("");
         TS_ASSERT(xml_helper::compare_xml(expected, xml));
     }
     
@@ -163,9 +162,11 @@ public:
         "    <definedNames/>"
         "    <calcPr calcId=\"124519\" fullCalcOnLoad=\"1\"/>"
         "</workbook>";
+        pugi::xml_document expected_xml;
+        expected_xml.load(expected.c_str());
 
-        TS_SKIP("");
-        TS_ASSERT(xml_helper::compare_xml(expected, xml));
+        TS_ASSERT(xml_helper::compare_xml(expected_xml.child("workbook").child("workbookPr"),
+            xml.child("workbook").child("workbookPr")));
     }
     
     void test_write_root_rels()

source/workbook/workbook.cpp

@@ -457,6 +457,7 @@ void workbook::clear()
     d_->relationships_.clear();
     d_->active_sheet_index_ = 0;
     d_->properties_ = document_properties();
+    d_->manifest_.clear();
     clear_styles();
     clear_formats();
 }

source/worksheet/tests/test_worksheet.hpp

@@ -30,7 +30,7 @@ public:
     {
         xlnt::workbook wb;
         xlnt::worksheet ws(wb);
-        TS_ASSERT_THROWS(xlnt::cell_reference invalid(xlnt::column_t((xlnt::column_t::index_t)0), 0), xlnt::value_error);
+        TS_ASSERT_THROWS(xlnt::cell_reference invalid(xlnt::column_t((xlnt::column_t::index_t)0), 0), xlnt::cell_coordinates_error);
     }
     
     void test_worksheet_dimension()

source/worksheet/worksheet.cpp

@@ -93,6 +93,23 @@ void worksheet::create_named_range(const std::string &name, const range_referenc
 {
     std::vector<named_range::target> targets;
     targets.push_back({ *this, reference });
+
+    try
+    {
+        auto temp = cell_reference::split_reference(name);
+
+        if (column_t(temp.first).index <= column_t("XFD").index || temp.second <= 1048576)
+        {
+            throw std::runtime_error("named range name must be outside the range A1-XFD1048576");
+        }
+        
+    }
+    catch(xlnt::error)
+    {
+        // not a valid cell reference
+    }
+    
+
     d_->named_ranges_[name] = named_range(name, targets);
 }
 
@@ -859,6 +876,11 @@ void worksheet::set_sheet_state(sheet_state state)
     get_page_setup().set_sheet_state(state);
 }
 
+sheet_state worksheet::get_sheet_state() const
+{
+    return get_page_setup().get_sheet_state();
+}
+
 void worksheet::add_column_properties(column_t column, const xlnt::column_properties &props)
 {
     d_->column_properties_[column] = props;

tests/helpers/path_helper.hpp

@@ -107,7 +107,19 @@ public:
     
     static std::string get_data_directory(const std::string &append = "")
     {
-        return get_executable_directory() + "../../tests/data" + append;
+        auto path = get_executable_directory() + "../../tests/data";
+
+        if (!append.empty())
+        {
+            if (append.front() != '/')
+            {
+                path.push_back('/');
+            }
+
+            path.append(append);
+        }
+
+        return path;
     }
     
     static void copy_file(const std::string &source, const std::string &destination, bool overwrite)