Merge pull request #2 from tfussell/dev

Dev

.travis.yml

@@ -1,35 +1,175 @@
-language: cpp
+# cpp takes longer
+language: minimal
 sudo: false
 dist: trusty
 
+git:
+  depth: false
+
 notifications:
   email: false
+  
+# set up build matrix
+matrix:
+  include:
+    # ============= GCC ==================
+    # gcc-6, c++11, debug build, dynamic linking
+    - os: linux
+      compiler: gcc
+      addons:
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+          packages:
+            - g++-6
+      env:
+        - CXX_COMPILER=g++-6
+        - C_COMPILER=gcc-6
+        - CXX_VER=11
+        - BUILD_TYPE=Release
+        - COVERAGE=OFF
+        - STATIC=OFF
+        - SAMPLES=OFF
+        
+    # gcc-7, c++14, release build, static linking, samples + benchmarks compiled
+    - os: linux
+      compiler: gcc
+      addons:
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+          packages:
+            - g++-7
+      env:
+        - CXX_COMPILER=g++-7
+        - C_COMPILER=gcc-7
+        - CXX_VER=14
+        - BUILD_TYPE=Release
+        - COVERAGE=OFF
+        - STATIC=ON
+        - SAMPLES=ON
+        
+    # gcc-8, c++17, release build, static linking, samples + benchmarks compiled
+    - os: linux
+      compiler: gcc
+      addons:
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+          packages:
+            - g++-8
+      env:
+        - CXX_COMPILER=g++-8
+        - C_COMPILER=gcc-8
+        - CXX_VER=17
+        - BUILD_TYPE=Release
+        - COVERAGE=OFF
+        - STATIC=ON
+        - SAMPLES=ON
 
-env:
+    # =========== CLANG =============
- - COVERAGE=ON STATIC=ON SAMPLES=OFF BENCHMARKS=OFF BUILD_TYPE=Debug
+    # clang 4, c++11, release build, dynamic linking
- - COVERAGE=OFF STATIC=ON SAMPLES=ON BENCHMARKS=OFF BUILD_TYPE=Debug
+    - os: linux
- - COVERAGE=OFF STATIC=OFF SAMPLES=ON BENCHMARKS=OFF BUILD_TYPE=Debug
+      compiler: clang
- - COVERAGE=OFF STATIC=ON SAMPLES=ON BENCHMARKS=ON BUILD_TYPE=Release
+      addons:
- - COVERAGE=OFF STATIC=OFF SAMPLES=ON BENCHMARKS=ON BUILD_TYPE=Release
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+            - llvm-toolchain-trusty-4.0
+          packages:
+            - clang-4.0
+      env:
+        - CXX_COMPILER=clang++-4.0
+        - C_COMPILER=clang-4.0
+        - CXX_VER=11
+        - BUILD_TYPE=Release
+        - COVERAGE=OFF
+        - STATIC=OFF
+        - SAMPLES=OFF
+        
+    # clang 5, c++14, release build, dynamic linking, samples + benchmarks compiled
+    - os: linux
+      compiler: clang
+      addons:
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+            - llvm-toolchain-trusty-5.0
+          packages:
+            - clang-5.0
+      env:
+        - CXX_COMPILER=clang++-5.0
+        - C_COMPILER=clang-5.0
+        - CXX_VER=14
+        - BUILD_TYPE=Release
+        - COVERAGE=OFF
+        - STATIC=ON
+        - SAMPLES=ON
+        
+    # clang 6, c++17, release build, static linking, samples compiled
+    - os: linux
+      compiler: clang
+      addons:
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+            - llvm-toolchain-trusty-6.0
+          packages:
+            - clang-6.0
+      env:
+        - CXX_COMPILER=clang++-6.0
+        - C_COMPILER=clang-6.0
+        - CXX_VER=17
+        - BUILD_TYPE=Release
+        - COVERAGE=OFF
+        - STATIC=ON
+        - SAMPLES=ON
 
-addons:
+    # ============= CODE COVERAGE ===============
-  apt:
+    # gcc-6, c++11, debug build, static linking, code coverage enabled
-    sources:
+    - os: linux
-     - ubuntu-toolchain-r-test
+      compiler: gcc
-    packages:
+      addons:
-     - g++-6 lcov
+        apt:
+          sources:
+            - ubuntu-toolchain-r-test
+          packages:
+            - g++-6
+            - lcov
+      env:
+        - CXX_COMPILER=g++-6
+        - C_COMPILER=gcc-6
+        - CXX_VER=11
+        - BUILD_TYPE=Debug
+        - COVERAGE=ON
+        - STATIC=ON
+        - SAMPLES=OFF
+        
+before_install:
+  - export CC=${C_COMPILER}
+  - export CXX=${CXX_COMPILER}
 
 install:
- - export CC=gcc-6
+  - |
- - export CXX=g++-6
+    if [[ "${COVERAGE}" == "ON" ]]; then
- - gem install coveralls-lcov
+      gem install coveralls-lcov; 
+    fi
+    
+  - ${CXX} --version
+  - cmake --version
 
 script:
- - mkdir build
+  - |
- - cd build
+    if [[ "${BUILD_TYPE}" == "Release" && "${STATIC}" == "ON" ]];
- - cmake -D STATIC=$STATIC -D BENCHMARKS=$BENCHMARKS -D SAMPLES=$SAMPLES -D COVERAGE=$COVERAGE -D CMAKE_BUILD_TYPE=$BUILD_TYPE ..
+      then BENCHMARKS="ON";
- - cmake --build .
+    else BENCHMARKS="OFF";
- - ./tests/xlnt.test
+    fi
+    
+  - mkdir build
+  - cd build
+  - cmake .. -DXLNT_CXX_LANG=${CXX_VER} -DSTATIC=$STATIC -DBENCHMARKS=$BENCHMARKS -DSAMPLES=$SAMPLES -DCOVERAGE=$COVERAGE -DCMAKE_BUILD_TYPE=$BUILD_TYPE
+  - cmake --build . -- -j2
+  - ./tests/xlnt.test
 
 after_success:
  - |

CMakeLists.txt

@@ -7,6 +7,18 @@ set(COMBINED_PROJECT TRUE)
 # Library type
 option(STATIC "Set to ON to build xlnt as a static library instead of a shared library" OFF)
 
+# c++ language standard to use
+set(XLNT_VALID_LANGS 11 14 17)
+set(XLNT_CXX_LANG "14" CACHE STRING "c++ language features to compile with")
+# enumerate allowed values for cmake gui
+set_property(CACHE XLNT_CXX_LANG PROPERTY STRINGS ${XLNT_VALID_LANGS})
+# validate value is in XLNT_VALID_LANGS
+list(FIND XLNT_VALID_LANGS ${XLNT_CXX_LANG} index)
+if(index EQUAL -1)
+    message(FATAL_ERROR "XLNT_CXX_LANG must be one of ${XLNT_VALID_LANGS}")
+endif()
+
+
 # Optional components
 option(TESTS "Set to OFF to skip building test executable (in ./tests)" ON)
 option(SAMPLES "Set to ON to build executable code samples (in ./samples)" OFF)

include/xlnt/cell/cell.hpp

@@ -262,26 +262,21 @@ public:
     /// </summary>
     class hyperlink hyperlink() const;
 
-    /// <summary>
-    /// Adds a hyperlink to this cell pointing to the URL of the given value.
-    /// </summary>
-    void hyperlink(const std::string &url);
-
     /// <summary>
     /// Adds a hyperlink to this cell pointing to the URI of the given value and sets
     /// the text value of the cell to the given parameter.
     /// </summary>
-    void hyperlink(const std::string &url, const std::string &display);
+    void hyperlink(const std::string &url, const std::string &display = "");
 
     /// <summary>
     /// Adds an internal hyperlink to this cell pointing to the given cell.
     /// </summary>
-    void hyperlink(xlnt::cell target);
+    void hyperlink(xlnt::cell target, const std::string& display = "");
 
     /// <summary>
     /// Adds an internal hyperlink to this cell pointing to the given range.
     /// </summary>
-    void hyperlink(xlnt::range target);
+    void hyperlink(xlnt::range target, const std::string& display = "");
 
     /// <summary>
     /// Returns true if this cell has a hyperlink set.
@@ -613,9 +608,9 @@ public:
     bool operator==(const cell &comparand) const;
 
     /// <summary>
-    /// Returns true if this cell is uninitialized.
+    /// Returns false if this cell the same cell as comparand (compared by reference).
     /// </summary>
-    bool operator==(std::nullptr_t) const;
+    bool operator!=(const cell &comparand) const;
 
 private:
     friend class style;
@@ -651,6 +646,11 @@ private:
 /// </summary>
 XLNT_API bool operator==(std::nullptr_t, const cell &cell);
 
+/// <summary>
+/// Returns true if this cell is uninitialized.
+/// </summary>
+XLNT_API bool operator==(const cell &cell, std::nullptr_t);
+
 /// <summary>
 /// Convenience function for writing cell to an ostream.
 /// Uses cell::to_string() internally.

include/xlnt/cell/hyperlink.hpp

@@ -44,12 +44,22 @@ class XLNT_API hyperlink
 public:
     bool external() const;
     class relationship relationship() const;
+    // external target
     std::string url() const;
+    // internal target
     std::string target_range() const;
+
+    bool has_display() const;
     void display(const std::string &value);
-    std::string display() const;
+    const std::string& display() const;
+
+    bool has_tooltip() const;
     void tooltip(const std::string &value);
-    std::string tooltip() const;
+    const std::string& tooltip() const;
+
+    bool has_location() const;
+    void location(const std::string &value);
+    const std::string& location() const;
 
 private:
     friend class cell;

include/xlnt/cell/rich_text_run.hpp

@@ -34,7 +34,7 @@ namespace xlnt {
 /// <summary>
 /// Typedef a rich_text_run as a pair of string and optional font.
 /// </summary>
-struct rich_text_run
+struct XLNT_API rich_text_run
 {
     std::string first;
     optional<font> second;

include/xlnt/packaging/relationship.hpp

@@ -115,7 +115,7 @@ public:
     /// <summary>
     /// Returns a string of the form rId# that identifies the relationship.
     /// </summary>
-    std::string id() const;
+    const std::string& id() const;
 
     /// <summary>
     /// Returns the type of this relationship.
@@ -130,12 +130,12 @@ public:
     /// <summary>
     /// Returns the URI of the package part this relationship points to.
     /// </summary>
-    uri source() const;
+    const uri &source() const;
 
     /// <summary>
     /// Returns the URI of the package part this relationship points to.
     /// </summary>
-    uri target() const;
+    const uri &target() const;
 
     /// <summary>
     /// Returns true if and only if rhs is equal to this relationship.

include/xlnt/packaging/uri.hpp

@@ -123,7 +123,7 @@ public:
     /// Returns the path of this URI.
     /// E.g. the path of http://example.com/document is "/document"
     /// </summary>
-    class path path() const;
+    const class path& path() const;
 
     /// <summary>
     /// Returns true if this URI has a non-null query string section.

include/xlnt/styles/color.hpp

@@ -252,19 +252,37 @@ public:
 	/// Returns the internal indexed color representing this color. If this is not an RGB color,
 	/// an invalid_attribute exception will be thrown.
     /// </summary>
-    rgb_color rgb() const;
+    const rgb_color& rgb() const;
+
+    /// <summary>
+    /// Returns the internal indexed color representing this color. If this is not an RGB color,
+    /// an invalid_attribute exception will be thrown.
+    /// </summary>
+    rgb_color &rgb();
 
     /// <summary>
     /// Returns the internal indexed color representing this color. If this is not an indexed color,
 	/// an invalid_attribute exception will be thrown.
     /// </summary>
-    indexed_color indexed() const;
+    const indexed_color& indexed() const;
+
+    /// <summary>
+    /// Returns the internal indexed color representing this color. If this is not an indexed color,
+    /// an invalid_attribute exception will be thrown.
+    /// </summary>
+    indexed_color &indexed();
 
     /// <summary>
 	/// Returns the internal indexed color representing this color. If this is not a theme color,
 	/// an invalid_attribute exception will be thrown.
     /// </summary>
-    theme_color theme() const;
+    const theme_color& theme() const;
+
+    /// <summary>
+    /// Returns the internal indexed color representing this color. If this is not a theme color,
+    /// an invalid_attribute exception will be thrown.
+    /// </summary>
+    theme_color& theme();
 
     /// <summary>
     /// Returns the tint of this color.

include/xlnt/utils/exceptions.hpp

@@ -120,7 +120,7 @@ public:
     /// <summary>
     /// Default constructor.
     /// </summary>
-    missing_number_format();
+    missing_number_format() = default;
 
     /// <summary>
     /// Default copy constructor.

include/xlnt/utils/optional.hpp

@@ -25,6 +25,7 @@
 
 #include <xlnt/xlnt_config.hpp>
 #include <xlnt/utils/exceptions.hpp>
+#include <type_traits>
 
 namespace xlnt {
 
@@ -34,39 +35,207 @@ namespace xlnt {
 /// within the optional class.
 /// </summary>
 template <typename T>
-class XLNT_API optional
+class optional
 {
+#if defined(_MSC_VER) && _MSC_VER <= 1900 // v14, visual studio 2015
+#define XLNT_NOEXCEPT_VALUE_COMPAT(...) (false)
+#else
+#define XLNT_NOEXCEPT_VALUE_COMPAT(...) (__VA_ARGS__)
+    using ctor_copy_T_noexcept = typename std::conditional<std::is_nothrow_copy_constructible<T>{}, std::true_type, std::false_type>::type;
+    using ctor_move_T_noexcept = typename std::conditional<std::is_nothrow_move_constructible<T>{}, std::true_type, std::false_type>::type;
+    using copy_ctor_noexcept = ctor_copy_T_noexcept;
+    using move_ctor_noexcept = ctor_move_T_noexcept;
+    using set_copy_noexcept_t = typename std::conditional<std::is_nothrow_copy_constructible<T>{} && std::is_nothrow_assignable<T, T>{}, std::true_type, std::false_type>::type;
+    using set_move_noexcept_t = typename std::conditional<std::is_nothrow_move_constructible<T>{} && std::is_nothrow_move_assignable<T>{}, std::true_type, std::false_type>::type;
+    using clear_noexcept_t = typename std::conditional<std::is_nothrow_destructible<T>{}, std::true_type, std::false_type>::type;
+#endif
+
 public:
     /// <summary>
     /// Default contructor. is_set() will be false initially.
     /// </summary>
-    optional() : has_value_(false), value_(T())
+    optional() noexcept
+        : has_value_(false)
     {
     }
 
     /// <summary>
     /// Constructs this optional with a value.
+    /// noexcept if T copy ctor is noexcept
     /// </summary>
-    optional(const T &value) : has_value_(true), value_(value)
+    optional(const T &value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(ctor_copy_T_noexcept{}))
+        : has_value_(true)
     {
+        new (&storage_) T(value);
+    }
+
+    /// <summary>
+    /// Constructs this optional with a value.
+    /// noexcept if T move ctor is noexcept
+    /// </summary>
+    optional(T &&value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(ctor_move_T_noexcept{}))
+        : has_value_(true)
+    {
+        new (&storage_) T(std::move(value));
+    }
+
+    /// <summary>
+    /// Copy constructs this optional from other
+    /// noexcept if T copy ctor is noexcept
+    /// </summary>
+    optional(const optional &other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(copy_ctor_noexcept{}))
+        : has_value_(other.has_value_)
+    {
+        if (has_value_)
+        {
+            new (&storage_) T(other.value_ref());
+        }
+    }
+
+    /// <summary>
+    /// Move constructs this optional from other. Clears the value from other if set
+    /// noexcept if T move ctor is noexcept
+    /// </summary>
+    optional(optional &&other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(move_ctor_noexcept{}))
+        : has_value_(other.has_value_)
+    {
+        if (has_value_)
+        {
+            new (&storage_) T(std::move(other.value_ref()));
+            other.clear();
+        }
+    }
+
+    /// <summary>
+    /// Copy assignment of this optional from other
+    /// noexcept if set and clear are noexcept for T&
+    /// </summary>
+    optional &operator=(const optional &other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{} && clear_noexcept_t{}))
+    {
+        if (other.has_value_)
+        {
+            set(other.value_ref());
+        }
+        else
+        {
+            clear();
+        }
+        return *this;
+    }
+
+    /// <summary>
+    /// Move assignment of this optional from other
+    /// noexcept if set and clear are noexcept for T&&
+    /// </summary>
+    optional &operator=(optional &&other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{} && clear_noexcept_t{}))
+    {
+        if (other.has_value_)
+        {
+            set(std::move(other.value_ref()));
+            other.clear();
+        }
+        else
+        {
+            clear();
+        }
+        return *this;
+    }
+
+    /// <summary>
+    /// Destructor cleans up the T instance if set
+    /// </summary>
+    ~optional() noexcept // note:: unconditional because msvc freaks out otherwise
+    {
+        clear();
     }
 
     /// <summary>
     /// Returns true if this object currently has a value set. This should
     /// be called before accessing the value with optional::get().
     /// </summary>
-    bool is_set() const
+    bool is_set() const noexcept
     {
         return has_value_;
     }
 
     /// <summary>
-    /// Sets the value to value.
+    /// Copies the value into the stored value
     /// </summary>
-    void set(const T &value)
+    void set(const T &value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{}))
     {
-        has_value_ = true;
+#if defined(__GNUC__) && !defined(__clang__)
-        value_ = value;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
+        if (has_value_)
+        {
+            value_ref() = value;
+        }
+        else
+        {
+            new (&storage_) T(value);
+            has_value_ = true;
+        }
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
+    }
+
+    /// <summary>
+    /// Moves the value into the stored value
+    /// </summary>
+    void set(T &&value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{}))
+    {
+        // note seperate overload for two reasons (as opposed to perfect forwarding)
+        // 1. have to deal with implicit conversions internally with perfect forwarding
+        // 2. have to deal with the noexcept specfiers for all the different variations
+        // overload is just far and away the simpler solution
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
+        if (has_value_)
+        {
+            value_ref() = std::move(value);
+        }
+        else
+        {
+            new (&storage_) T(std::move(value));
+            has_value_ = true;
+        }
+#if defined(__GNUC__) && !defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
+    }
+
+    /// <summary>
+    /// Assignment operator overload. Equivalent to setting the value using optional::set.
+    /// </summary>
+    optional &operator=(const T &rhs) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{}))
+    {
+        set(rhs);
+        return *this;
+    }
+
+    /// <summary>
+    /// Assignment operator overload. Equivalent to setting the value using optional::set.
+    /// </summary>
+    optional &operator=(T &&rhs) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{}))
+    {
+        set(std::move(rhs));
+        return *this;
+    }
+
+    /// <summary>
+    /// After this is called, is_set() will return false until a new value is provided.
+    /// </summary>
+    void clear() noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(clear_noexcept_t{}))
+    {
+        if (has_value_)
+        {
+            reinterpret_cast<T *>(&storage_)->~T();
+        }
+        has_value_ = false;
     }
 
     /// <summary>
@@ -80,7 +249,7 @@ public:
             throw invalid_attribute();
         }
 
-        return value_;
+        return value_ref();
     }
 
     /// <summary>
@@ -94,28 +263,7 @@ public:
             throw invalid_attribute();
         }
 
-        return value_;
+        return value_ref();
-    }
-
-    /// <summary>
-    /// Resets the internal value using its default constructor. After this is
-    /// called, is_set() will return false until a new value is provided.
-    /// </summary>
-    void clear()
-    {
-        has_value_ = false;
-        value_ = T();
-    }
-
-    /// <summary>
-    /// Assignment operator. Equivalent to setting the value using optional::set.
-    /// </summary>
-    optional &operator=(const T &rhs)
-    {
-        has_value_ = true;
-        value_ = rhs;
-
-        return *this;
     }
 
     /// <summary>
@@ -123,21 +271,47 @@ public:
     /// or both have a value and those values are equal according to
     /// their equality operator.
     /// </summary>
-    bool operator==(const optional<T> &other) const
+    bool operator==(const optional<T> &other) const noexcept
     {
-        if (has_value_ != other.has_value_) {
+        if (has_value_ != other.has_value_)
+        {
             return false;
         }
         if (!has_value_)
         {
             return true;
         }
-        return value_ == other.value_;
+        return value_ref() == other.value_ref();
+    }
+
+    /// <summary>
+    /// Returns false if neither this nor other have a value
+    /// or both have a value and those values are equal according to
+    /// their equality operator.
+    /// </summary>
+    bool operator!=(const optional<T> &other) const noexcept
+    {
+        return !operator==(other);
     }
 
 private:
+    // helpers for getting a T out of storage
+    T &value_ref() noexcept
+    {
+        return *reinterpret_cast<T *>(&storage_);
+    }
+
+    const T &value_ref() const noexcept
+    {
+        return *reinterpret_cast<const T *>(&storage_);
+    }
+
     bool has_value_;
-    T value_;
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type storage_;
 };
 
-} // namespace xlnt
+#ifdef XLNT_NOEXCEPT_VALUE_COMPAT
+#undef XLNT_NOEXCEPT_VALUE_COMPAT
+#endif
+
+} // namespace xlnt

include/xlnt/utils/path.hpp

@@ -107,7 +107,7 @@ public:
     /// Create a string representing this path separated by the provided
     /// separator or the system-default separator if not provided.
     /// </summary>
-    std::string string() const;
+    const std::string& string() const;
 
 #ifdef _MSC_VER
     /// <summary>
@@ -176,6 +176,11 @@ public:
     /// </summary>
     bool operator==(const path &other) const;
 
+    /// <summary>
+    /// Returns true if left path is equal to right path.
+    /// </summary>
+    bool operator!=(const path &other) const;
+
 private:
     /// <summary>
     /// Returns the character that separates directories in the path.

include/xlnt/workbook/document_security.hpp

@@ -67,7 +67,7 @@ public:
     /// <summary>
     /// Constructs a new document security object with default values.
     /// </summary>
-    document_security();
+    document_security() = default;
 
     /// <summary>
     /// If true, the workbook is locked for revisions.

include/xlnt/worksheet/phonetic_pr.hpp

@@ -38,7 +38,7 @@ namespace xlnt {
 class XLNT_API phonetic_pr
 {
 public:
-    static const std::string Serialised_ID;
+    static std::string Serialised_ID();
 
     /// <summary>
     /// possible values for alignment property

include/xlnt/worksheet/range.hpp

@@ -290,11 +290,11 @@ public:
     /// </summary>
     bool operator!=(const range &comparand) const;
 
-private:
+ private:
     /// <summary>
     /// The worksheet this range is within
     /// </summary>
-    worksheet ws_;
+    class worksheet ws_;
 
     /// <summary>
     /// The reference of this range

include/xlnt/worksheet/range_reference.hpp

@@ -56,12 +56,7 @@ public:
     /// top_left:bottom_right.
     /// </summary>
     explicit range_reference(const char *range_string);
-
+    
-    /// <summary>
-    /// Constructs a range reference from a pair of cell references.
-    /// </summary>
-    explicit range_reference(const std::pair<cell_reference, cell_reference> &reference_pair);
-
     /// <summary>
     /// Constructs a range reference from cell references indicating top
     /// left and bottom right coordinates of the range.

source/CMakeLists.txt

@@ -1,11 +1,9 @@
 cmake_minimum_required(VERSION 3.1)
 project(xlnt VERSION 1.2)
 
-# Require C99 and C++11 compilers
+set(CMAKE_CXX_STANDARD ${XLNT_CXX_LANG})
-set(CMAKE_C_STANDARD 99)
-set(CMAKE_C_STANDARD_REQUIRED ON)
-set(CMAKE_CXX_STANDARD 11)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CXX_EXTENSIONS OFF)
 
 # Project metadata
 set(PROJECT_VENDOR "Thomas Fussell")
@@ -37,12 +35,18 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-pragmas") # ignore MSVC and Clang pragmas
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything") # all warnings
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror") # warnings are errors
+  # blacklist warnings that are not relevant
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-c++98-compat") # ignore warnings about C++98 compatibility
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-c++98-compat-pedantic") # ignore pedantic warnings about C++98 compatibility
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-padded") # ignore padding warnings
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-documentation-unknown-command") # ignore unknown commands in Javadoc-style comments
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-pragmas") # ignore Windows and GCC pragmas
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-warning-option") # ignore Windows and GCC pragmas
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-float-equal") # don't warn on uses of == for fp types
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-newline-eof") # no longer an issue with post-c++11 standards which mandate include add a newline if neccesary
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-covered-switch-default") # default is often added to switches for completeness or to cover future alternatives
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-exit-time-destructors") # this is just a warning to notify that the destructor will run during exit
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-braces") # Wmissing-field-initializers has less false positives
 endif()
 
 if(STATIC_CRT)
@@ -151,6 +155,9 @@ else()
   target_compile_definitions(xlnt PUBLIC XLNT_STATIC=1)
 endif()
 
+# requires cmake 3.8+
+#target_compile_features(xlnt PUBLIC cxx_std_${XLNT_CXX_LANG})
+
 # Includes
 target_include_directories(xlnt PUBLIC ${XLNT_INCLUDE_DIR})
 target_include_directories(xlnt PRIVATE ${XLNT_SOURCE_DIR})
@@ -173,23 +180,23 @@ if(MSVC)
   set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/cryptography/aes.cpp
     PROPERTIES
     COMPILE_FLAGS "/wd\"4996\"")
-endif()
+else()
-
+  # Platform- and file-specific settings, Clang
-# Platform- and file-specific settings, Clang
+  if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+    set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/serialization/miniz.cpp
-  set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/serialization/miniz.cpp
+	  PROPERTIES
-    PROPERTIES
+	  COMPILE_FLAGS "-Wno-undef")
-    COMPILE_FLAGS "-Wno-undef")
+    set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/serialization/zstream.cpp
-  set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/serialization/zstream.cpp
+	  PROPERTIES
-    PROPERTIES
+      COMPILE_FLAGS "-Wno-undef -Wno-shorten-64-to-32")
-    COMPILE_FLAGS "-Wno-undef -Wno-shorten-64-to-32")
+  endif()
-endif()
+  
-
+  # Platform- and file-specific settings, GCC
-# Platform- and file-specific settings, GCC
+  if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
-if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+    set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/serialization/miniz.cpp
-  set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/detail/serialization/miniz.cpp
+	  PROPERTIES
-    PROPERTIES
+	  COMPILE_FLAGS "-Wno-strict-aliasing")
-    COMPILE_FLAGS "-Wno-strict-aliasing")
+  endif()
 endif()
 
 # Group files into pseudo-folders in IDEs

source/cell/cell.cpp

@@ -341,38 +341,24 @@ cell_reference cell::reference() const
     return {d_->column_, d_->row_};
 }
 
-bool cell::operator==(std::nullptr_t) const
-{
-    return d_ == nullptr;
-}
-
 bool cell::operator==(const cell &comparand) const
 {
     return d_ == comparand.d_;
 }
 
-cell &cell::operator=(const cell &rhs)
+bool cell::operator!=(const cell &comparand) const
 {
-    d_->column_ = rhs.d_->column_;
+    return d_ != comparand.d_;
-    d_->format_ = rhs.d_->format_;
-    d_->formula_ = rhs.d_->formula_;
-    d_->hyperlink_ = rhs.d_->hyperlink_;
-    d_->is_merged_ = rhs.d_->is_merged_;
-    d_->parent_ = rhs.d_->parent_;
-    d_->row_ = rhs.d_->row_;
-    d_->type_ = rhs.d_->type_;
-    d_->value_numeric_ = rhs.d_->value_numeric_;
-    d_->value_text_ = rhs.d_->value_text_;
-
-    return *this;
 }
 
+cell &cell::operator=(const cell &rhs) = default;
+
 hyperlink cell::hyperlink() const
 {
     return xlnt::hyperlink(&d_->hyperlink_.get());
 }
 
-void cell::hyperlink(const std::string &url)
+void cell::hyperlink(const std::string &url, const std::string &display)
 {
     if (url.empty() || std::find(url.begin(), url.end(), ':') == url.end())
     {
@@ -388,7 +374,8 @@ void cell::hyperlink(const std::string &url)
     auto relationships = manifest.relationships(ws.path(), relationship_type::hyperlink);
     auto relation = std::find_if(relationships.cbegin(), relationships.cend(),
         [&url](xlnt::relationship rel) { return rel.target().path().string() == url; });
-    if (relation != relationships.end()) {
+    if (relation != relationships.end())
+    {
         d_->hyperlink_.get().relationship = *relation;
     }
     else
@@ -400,29 +387,20 @@ void cell::hyperlink(const std::string &url)
             target_mode::external);
         // TODO: make manifest::register_relationship return the created relationship instead of rel id
         d_->hyperlink_.get().relationship = manifest.relationship(ws.path(), rel_id);
-    }   
+    }
-    
+    // if a value is already present, the display string is ignored
-    if (!has_value()) // hyperlink on an empty cell sets the value to the hyperlink string
+    if (has_value())
     {
-        value(url);
+        d_->hyperlink_.get().display.set(to_string());
+    }
+    else
+    {
+        d_->hyperlink_.get().display.set(display.empty() ? url : display);
+        value(hyperlink().display());
     }
 }
 
-void cell::hyperlink(const std::string &url, const std::string &display)
+void cell::hyperlink(xlnt::cell target, const std::string& display)
-{
-    if (!display.empty()) // if the display string isn't empty use that
-    {
-        value(display);
-    }
-    else // empty display string sets the value to the link text
-    {
-        value(url);
-    }
-    hyperlink(url);
-    d_->hyperlink_.get().display = display;
-}
-
-void cell::hyperlink(xlnt::cell target)
 {
     // TODO: should this computed value be a method on a cell?
     const auto cell_address = target.worksheet().title() + "!" + target.reference().to_string();
@@ -430,8 +408,37 @@ void cell::hyperlink(xlnt::cell target)
     d_->hyperlink_ = detail::hyperlink_impl();
     d_->hyperlink_.get().relationship = xlnt::relationship("", relationship_type::hyperlink,
         uri(""), uri(cell_address), target_mode::internal);
-    d_->hyperlink_.get().display = cell_address;
+    // if a value is already present, the display string is ignored
-    value(cell_address);
+    if (has_value())
+    {
+        d_->hyperlink_.get().display.set(to_string());
+    }
+    else
+    {
+        d_->hyperlink_.get().display.set(display.empty() ? cell_address : display);
+        value(hyperlink().display());
+    }
+}
+
+void cell::hyperlink(xlnt::range target, const std::string &display)
+{
+    // TODO: should this computed value be a method on a cell?
+    const auto range_address = target.target_worksheet().title() + "!" + target.reference().to_string();
+
+    d_->hyperlink_ = detail::hyperlink_impl();
+    d_->hyperlink_.get().relationship = xlnt::relationship("", relationship_type::hyperlink,
+        uri(""), uri(range_address), target_mode::internal);
+    
+    // if a value is already present, the display string is ignored
+    if (has_value())
+    {
+        d_->hyperlink_.get().display.set(to_string());
+    }
+    else
+    {
+        d_->hyperlink_.get().display.set(display.empty() ? range_address : display);
+        value(hyperlink().display());
+    }
 }
 
 void cell::formula(const std::string &formula)
@@ -472,6 +479,15 @@ void cell::clear_formula()
     }
 }
 
+std::string cell::error() const
+{
+    if (d_->type_ != type::error)
+    {
+        throw xlnt::exception("called error() when cell type is not error");
+    }
+    return value<std::string>();
+}
+
 void cell::error(const std::string &error)
 {
     if (error.length() == 0 || error[0] != '#')
@@ -743,6 +759,16 @@ calendar cell::base_date() const
     return workbook().base_date();
 }
 
+bool operator==(std::nullptr_t, const cell &cell)
+{
+    return cell.data_type() == cell::type::empty;
+}
+
+bool operator==(const cell &cell, std::nullptr_t)
+{
+    return nullptr == cell;
+}
+
 XLNT_API std::ostream &operator<<(std::ostream &stream, const xlnt::cell &cell)
 {
     return stream << cell.to_string();
@@ -802,8 +828,11 @@ void cell::value(const std::string &value_string, bool infer_type)
 
 void cell::clear_format()
 {
-    format().d_->references -= format().d_->references > 0 ? 1 : 0;
+    if (d_->format_.is_set())
-    d_->format_.clear();
+    {
+        format().d_->references -= format().d_->references > 0 ? 1 : 0;
+        d_->format_.clear();
+    }
 }
 
 void cell::clear_style()

source/cell/hyperlink.cpp

@@ -67,24 +67,49 @@ bool hyperlink::external() const
     return d_->relationship.target_mode() == target_mode::external;
 }
 
-void hyperlink::display(const std::string &value)
+bool hyperlink::has_display() const
 {
-    d_->display = value;
+    return d_->display.is_set();
 }
 
-std::string hyperlink::display() const
+void hyperlink::display(const std::string &value)
 {
-    return d_->display;
+    d_->display.set(value);
+}
+
+const std::string& hyperlink::display() const
+{
+    return d_->display.get();
+}
+
+bool hyperlink::has_tooltip() const
+{
+    return d_->tooltip.is_set();
 }
 
 void hyperlink::tooltip(const std::string &value)
 {
-    d_->tooltip = value;
+    d_->tooltip.set(value);
 }
 
-std::string hyperlink::tooltip() const
+const std::string& hyperlink::tooltip() const
 {
-    return d_->tooltip;
+    return d_->tooltip.get();
+}
+
+bool hyperlink::has_location() const
+{
+    return d_->location.is_set();
+}
+
+void hyperlink::location(const std::string &value)
+{
+    d_->location.set(value);
+}
+
+const std::string &hyperlink::location() const
+{
+    return d_->location.get();
 }
 
 } // namespace xlnt

source/cell/rich_text.cpp

@@ -26,15 +26,22 @@
 #include <xlnt/cell/rich_text.hpp>
 #include <xlnt/cell/rich_text_run.hpp>
 
+namespace {
+bool has_trailing_whitespace(const std::string &s) 
+{
+    return !s.empty() && (s.front() == ' ' || s.back() == ' ');
+};
+}
+
 namespace xlnt {
 
 rich_text::rich_text(const std::string &plain_text)
-    : rich_text(rich_text_run{plain_text, optional<font>(), false})
+    : rich_text(rich_text_run{plain_text, optional<font>(), has_trailing_whitespace(plain_text)})
 {
 }
 
 rich_text::rich_text(const std::string &plain_text, const class font &text_font)
-    : rich_text(rich_text_run{plain_text, optional<font>(text_font), false})
+    : rich_text(rich_text_run{plain_text, optional<font>(text_font), has_trailing_whitespace(plain_text)})
 {
 }
 
@@ -65,6 +72,11 @@ std::vector<rich_text_run> rich_text::runs() const
     return runs_;
 }
 
+void rich_text::runs(const std::vector<rich_text_run> &new_runs)
+{
+    runs_ = new_runs;
+}
+
 void rich_text::add_run(const rich_text_run &t)
 {
     runs_.push_back(t);

source/detail/implementations/cell_impl.hpp

@@ -27,22 +27,26 @@
 
 #include <xlnt/cell/cell_type.hpp>
 #include <xlnt/cell/comment.hpp>
-#include <xlnt/cell/rich_text.hpp>
 #include <xlnt/cell/index_types.hpp>
+#include <xlnt/cell/rich_text.hpp>
 #include <xlnt/packaging/relationship.hpp>
 #include <xlnt/utils/optional.hpp>
+#include <detail/implementations/format_impl.hpp>
 #include <detail/implementations/hyperlink_impl.hpp>
 
 namespace xlnt {
 namespace detail {
 
-struct format_impl;
 struct worksheet_impl;
 
 struct cell_impl
 {
     cell_impl();
-
+    cell_impl(const cell_impl &other) = default;
+    cell_impl(cell_impl &&other) = default;
+    cell_impl &operator=(const cell_impl &other) = default;
+    cell_impl &operator=(cell_impl &&other) = default;
+    
     cell_type type_;
 
     worksheet_impl *parent_;
@@ -72,9 +76,8 @@ inline bool operator==(const cell_impl &lhs, const cell_impl &rhs)
         && lhs.value_numeric_ == rhs.value_numeric_
         && lhs.formula_ == rhs.formula_
         && lhs.hyperlink_ == rhs.hyperlink_
-        // comparing pointers is unlikely to be what is wanted
+        && (lhs.format_.is_set() == rhs.format_.is_set() && (!lhs.format_.is_set() || *lhs.format_.get() == *rhs.format_.get()))
-        /*&& lhs.format_ == rhs.format_
+        && (lhs.comment_.is_set() == rhs.comment_.is_set() && (!lhs.comment_.is_set() || *lhs.comment_.get() == *rhs.comment_.get()));
-        && lhs.comment_ == rhs.comment_*/;
 }
 
 } // namespace detail

source/detail/implementations/hyperlink_impl.hpp

@@ -26,15 +26,18 @@
 #include <string>
 
 #include <xlnt/packaging/relationship.hpp>
+#include <xlnt/utils/optional.hpp>
 
 namespace xlnt {
 namespace detail {
 
+// [serialised]
 struct hyperlink_impl
 {
     xlnt::relationship relationship;
-    std::string tooltip;
+    xlnt::optional<std::string> location;
-    std::string display;
+    xlnt::optional<std::string> tooltip;
+    xlnt::optional<std::string> display;
 };
 
 inline bool operator==(const hyperlink_impl &lhs, const hyperlink_impl &rhs)

source/detail/serialization/xlsx_consumer.cpp

@@ -387,7 +387,7 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
                 props.sync_horizontal.set(parser().attribute<bool>("syncHorizontal"));
             }
             if (parser().attribute_present("syncVertical"))
-            {// optional, boolean, false
+            { // optional, boolean, false
                 props.sync_vertical.set(parser().attribute<bool>("syncVertical"));
             }
             if (parser().attribute_present("syncRef"))
@@ -399,11 +399,11 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
                 props.transition_evaluation.set(parser().attribute<bool>("transitionEvaluation"));
             }
             if (parser().attribute_present("transitionEntry"))
-            {// optional, boolean, false
+            { // optional, boolean, false
                 props.transition_entry.set(parser().attribute<bool>("transitionEntry"));
             }
             if (parser().attribute_present("published"))
-            {// optional, boolean, true
+            { // optional, boolean, true
                 props.published.set(parser().attribute<bool>("published"));
             }
             if (parser().attribute_present("codeName"))
@@ -411,11 +411,11 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
                 props.code_name.set(parser().attribute<std::string>("codeName"));
             }
             if (parser().attribute_present("filterMode"))
-            {// optional, boolean, false
+            { // optional, boolean, false
                 props.filter_mode.set(parser().attribute<bool>("filterMode"));
             }
             if (parser().attribute_present("enableFormatConditionsCalculation"))
-            {// optional, boolean, true
+            { // optional, boolean, true
                 props.enable_format_condition_calculation.set(parser().attribute<bool>("enableFormatConditionsCalculation"));
             }
             ws.d_->sheet_properties_.set(props);
@@ -606,19 +606,22 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
                 auto min = static_cast<column_t::index_t>(std::stoull(parser().attribute("min")));
                 auto max = static_cast<column_t::index_t>(std::stoull(parser().attribute("max")));
 
-                optional<double> width;
+                // avoid uninitialised warnings in GCC by using a lambda to make the conditional initialisation
-
+                optional<double> width = [](xml::parser &p) -> xlnt::optional<double> {
-                if (parser().attribute_present("width"))
+                    if (p.attribute_present("width"))
-                {
+                    {
-                    width = (parser().attribute<double>("width") * 7 - 5) / 7;
+                        return (p.attribute<double>("width") * 7 - 5) / 7;
-                }
+                    }
-
+                    return xlnt::optional<double>();
-                optional<std::size_t> column_style;
+                }(parser());
-
+                // avoid uninitialised warnings in GCC by using a lambda to make the conditional initialisation
-                if (parser().attribute_present("style"))
+                optional<std::size_t> column_style = [](xml::parser &p) -> xlnt::optional<std::size_t> {
-                {
+                    if (p.attribute_present("style"))
-                    column_style = parser().attribute<std::size_t>("style");
+                    {
-                }
+                        return p.attribute<std::size_t>("style");
+                    }
+                    return xlnt::optional<std::size_t>();
+                }(parser());
 
                 auto custom = parser().attribute_present("customWidth")
                     ? is_true(parser().attribute("customWidth"))
@@ -1843,7 +1846,8 @@ void xlsx_consumer::read_office_document(const std::string &content_type) // CT_
             target_.d_->sheet_title_rel_id_map_.end(),
             [&](const std::pair<std::string, std::string> &p) {
                 return p.second == worksheet_rel.id();
-            })->first;
+            })
+                         ->first;
 
         auto id = sheet_title_id_map_[title];
         auto index = sheet_title_index_map_[title];

source/detail/serialization/xlsx_producer.cpp

@@ -816,7 +816,6 @@ void xlsx_producer::write_pivot_table(const relationship & /*rel*/)
 void xlsx_producer::write_shared_string_table(const relationship & /*rel*/)
 {
     static const auto &xmlns = constants::ns("spreadsheetml");
-    static const auto &xmlns_xml = constants::ns("xml");
 
     write_start_element(xmlns, "sst");
     write_namespace(xmlns, "");
@@ -853,18 +852,13 @@ void xlsx_producer::write_shared_string_table(const relationship & /*rel*/)
     write_attribute("count", string_count);
     write_attribute("uniqueCount", source_.shared_strings().size());
 
-    auto has_trailing_whitespace = [](const std::string &s)
-    {
-        return !s.empty() && (s.front() == ' ' || s.back() == ' ');
-    };
-
     for (const auto &string : source_.shared_strings())
     {
         if (string.runs().size() == 1 && !string.runs().at(0).second.is_set())
         {
             write_start_element(xmlns, "si");
             write_start_element(xmlns, "t");
-            write_characters(string.plain_text(),string.runs().front().preserve_space);
+            write_characters(string.plain_text(), string.runs().front().preserve_space);
             write_end_element(xmlns, "t");
             write_end_element(xmlns, "si");
 
@@ -926,7 +920,7 @@ void xlsx_producer::write_shared_string_table(const relationship & /*rel*/)
             }
 
             write_start_element(xmlns, "t");
-            write_characters(run.first, has_trailing_whitespace(run.first));
+            write_characters(run.first, run.preserve_space);
             write_end_element(xmlns, "t");
             write_end_element(xmlns, "r");
         }
@@ -2755,7 +2749,7 @@ void xlsx_producer::write_worksheet(const relationship &rel)
 
     if (ws.has_phonetic_properties())
     {
-        write_start_element(xmlns, phonetic_pr::Serialised_ID);
+        write_start_element(xmlns, phonetic_pr::Serialised_ID());
         const auto &ph_props = ws.phonetic_properties();
         write_attribute("fontId", ph_props.font_id());
         if (ph_props.has_type())
@@ -2766,7 +2760,7 @@ void xlsx_producer::write_worksheet(const relationship &rel)
         {
             write_attribute("alignment", phonetic_pr::alignment_as_string(ph_props.alignment()));
         }
-        write_end_element(xmlns, phonetic_pr::Serialised_ID);
+        write_end_element(xmlns, phonetic_pr::Serialised_ID());
     }
 
     if (ws.has_page_margins())

source/detail/serialization/zstream.cpp

@@ -366,7 +366,7 @@ public:
             deflateEnd(&strm);
             if (header)
             {
-                std::ios::streampos final_position = ostream.tellp();
+                auto final_position = ostream.tellp();
                 header->uncompressed_size = uncompressed_size;
                 header->crc = crc;
                 ostream.seekp(header->header_offset);
@@ -457,14 +457,14 @@ ozstream::ozstream(std::ostream &stream)
 ozstream::~ozstream()
 {
     // Write all file headers
-    std::ios::streampos final_position = destination_stream_.tellp();
+    auto final_position = destination_stream_.tellp();
 
     for (const auto &header : file_headers_)
     {
         write_header(header, destination_stream_, true);
     }
 
-    std::ios::streampos central_end = destination_stream_.tellp();
+    auto central_end = destination_stream_.tellp();
 
     // Write end of central
     write_int(destination_stream_, static_cast<std::uint32_t>(0x06054b50)); // end of central
@@ -507,12 +507,12 @@ bool izstream::read_central_header()
     // Find the header
     // NOTE: this assumes the zip file header is the last thing written to file...
     source_stream_.seekg(0, std::ios_base::end);
-    std::ios::streampos end_position = source_stream_.tellg();
+    auto end_position = static_cast<std::size_t>(source_stream_.tellg());
 
     auto max_comment_size = std::uint32_t(0xffff); // max size of header
     auto read_size_before_comment = std::uint32_t(22);
 
-    std::ios::streamoff read_start = max_comment_size + read_size_before_comment;
+    std::uint32_t read_start = max_comment_size + read_size_before_comment;
 
     if (read_start > end_position)
     {

source/packaging/relationship.cpp

@@ -36,7 +36,7 @@ relationship::relationship(
 {
 }
 
-std::string relationship::id() const
+const std::string& relationship::id() const
 {
     return id_;
 }
@@ -46,12 +46,12 @@ target_mode relationship::target_mode() const
     return mode_;
 }
 
-uri relationship::source() const
+const uri &relationship::source() const
 {
     return source_;
 }
 
-uri relationship::target() const
+const uri &relationship::target() const
 {
     return target_;
 }

source/packaging/uri.cpp

@@ -16,7 +16,7 @@ std::string uri::to_string() const
     return path_.string();
 }
 
-path uri::path() const
+const path& uri::path() const
 {
     return path_;
 }

source/styles/color.cpp

@@ -57,6 +57,11 @@ std::size_t indexed_color::index() const
 	return index_;
 }
 
+void indexed_color::index(std::size_t index)
+{
+    index_ = index;
+}
+
 // theme_color implementation
 
 theme_color::theme_color(std::size_t index) : index_(index)
@@ -68,6 +73,11 @@ std::size_t theme_color::index() const
 	return index_;
 }
 
+void theme_color::index(std::size_t index)
+{
+    index_ = index;
+}
+
 // rgb_color implementation
 
 std::string rgb_color::hex_string() const
@@ -223,19 +233,37 @@ void color::auto_(bool value)
 	auto__ = value;
 }
 
-indexed_color color::indexed() const
+const indexed_color& color::indexed() const
 {
 	assert_type(color_type::indexed);
 	return indexed_;
 }
 
-theme_color color::theme() const
+indexed_color &color::indexed()
+{
+    assert_type(color_type::indexed);
+    return indexed_;
+}
+
+const theme_color& color::theme() const
 {
 	assert_type(color_type::theme);
 	return theme_;
 }
 
-rgb_color color::rgb() const
+theme_color &color::theme()
+{
+    assert_type(color_type::theme);
+    return theme_;
+}
+
+const rgb_color& color::rgb() const
+{
+    assert_type(color_type::rgb);
+    return rgb_;
+}
+
+rgb_color &color::rgb()
 {
     assert_type(color_type::rgb);
     return rgb_;

source/styles/conditional_format.cpp

@@ -81,6 +81,11 @@ bool conditional_format::operator!=(const conditional_format &other) const
 	return !(*this == other);
 }
 
+bool conditional_format::has_border() const
+{
+    return d_->border_id.is_set();
+}
+
 xlnt::border conditional_format::border() const
 {
     return d_->parent->borders.at(d_->border_id.get());
@@ -92,6 +97,11 @@ conditional_format conditional_format::border(const xlnt::border &new_border)
 	return *this;
 }
 
+bool conditional_format::has_fill() const
+{
+    return d_->fill_id.is_set();
+}
+
 xlnt::fill conditional_format::fill() const
 {
     return d_->parent->fills.at(d_->fill_id.get());
@@ -103,6 +113,11 @@ conditional_format conditional_format::fill(const xlnt::fill &new_fill)
 	return *this;
 }
 
+bool conditional_format::has_font() const
+{
+    return d_->font_id.is_set();
+}
+
 xlnt::font conditional_format::font() const
 {
     return d_->parent->fonts.at(d_->font_id.get());

source/styles/font.cpp

@@ -27,7 +27,11 @@
 #include <xlnt/styles/font.hpp>
 
 namespace {
-const std::string Default_Name = "Calibri";
+const std::string &Default_Name()
+{
+    static const std::string Default("Calibri");
+    return Default;
+}
 constexpr double Default_Size = 12.0;
 } // namespace
 
@@ -161,13 +165,13 @@ font &font::name(const std::string &name)
     return *this;
 }
 
-const std::string& font::name() const
+const std::string &font::name() const
 {
     if (name_.is_set())
     {
         return name_.get();
     }
-    return Default_Name;
+    return Default_Name();
 }
 
 bool font::has_color() const
@@ -229,7 +233,7 @@ std::size_t font::family() const
     return family_.get();
 }
 
-const std::string& font::scheme() const
+const std::string &font::scheme() const
 {
     return scheme_.get();
 }

source/styles/protection.cpp

@@ -26,6 +26,26 @@
 
 namespace xlnt {
 
+protection protection::unlocked_and_visible()
+{
+    return protection();
+}
+
+protection protection::locked_and_visible()
+{
+    return protection().locked(true);
+}
+
+protection protection::unlocked_and_hidden()
+{
+    return protection().hidden(true);
+}
+
+protection protection::locked_and_hidden()
+{
+    return protection().locked(true).hidden(true);
+}
+
 protection::protection()
     : locked_(false), hidden_(false)
 {

source/styles/style.cpp

@@ -66,6 +66,11 @@ std::size_t style::builtin_id() const
     return d_->builtin_id.get();
 }
 
+bool style::builtin() const
+{
+    return d_->builtin_id.is_set();
+}
+
 std::string style::name() const
 {
     return d_->name;
@@ -87,6 +92,11 @@ bool style::operator==(const style &other) const
     return name() == other.name();
 }
 
+bool style::operator!=(const style &other) const
+{
+    return !operator==(other);
+}
+
 xlnt::alignment style::alignment() const
 {
     return d_->parent->alignments.at(d_->alignment_id.get());

source/utils/path.cpp

@@ -141,6 +141,22 @@ path::path(const std::string &path_string)
 {
 }
 
+path::path(const std::string &path_string, char sep)
+    : internal_(path_string)
+{
+    char curr_sep = guess_separator();
+    if (curr_sep != sep)
+    {
+        for (char& c : internal_) // simple find and replace
+        {
+            if (c == curr_sep)
+            {
+                c = sep;
+            }
+        }
+    }
+}
+
 // general attributes
 
 bool path::is_relative() const
@@ -205,7 +221,7 @@ std::pair<std::string, std::string> path::split_extension() const
 
 // conversion
 
-std::string path::string() const
+const std::string& path::string() const
 {
     return internal_;
 }
@@ -331,4 +347,9 @@ bool path::operator==(const path &other) const
     return internal_ == other.internal_;
 }
 
+bool path::operator!=(const path &other) const
+{
+    return !operator==(other);
+}
+
 } // namespace xlnt

source/utils/variant.cpp

@@ -27,9 +27,8 @@
 namespace xlnt {
 
 variant::variant()
-{
+    : type_(type::null)
-
+{}
-}
 
 variant::variant(const std::string &value)
     : type_(type::lpstr),
@@ -41,7 +40,7 @@ variant::variant(const char *value) : variant(std::string(value))
 {
 }
 
-variant::variant(std::int32_t value)
+variant::variant(int32_t value)
     : type_(type::i4),
     i4_value_(value)
 {

source/workbook/workbook.cpp

@@ -28,16 +28,6 @@
 #include <functional>
 #include <set>
 
-#include <detail/constants.hpp>
-#include <detail/default_case.hpp>
-#include <detail/implementations/cell_impl.hpp>
-#include <detail/implementations/workbook_impl.hpp>
-#include <detail/implementations/worksheet_impl.hpp>
-#include <detail/serialization/excel_thumbnail.hpp>
-#include <detail/serialization/vector_streambuf.hpp>
-#include <detail/serialization/open_stream.hpp>
-#include <detail/serialization/xlsx_consumer.hpp>
-#include <detail/serialization/xlsx_producer.hpp>
 #include <xlnt/cell/cell.hpp>
 #include <xlnt/packaging/manifest.hpp>
 #include <xlnt/packaging/relationship.hpp>
@@ -61,12 +51,22 @@
 #include <xlnt/worksheet/header_footer.hpp>
 #include <xlnt/worksheet/range.hpp>
 #include <xlnt/worksheet/worksheet.hpp>
+#include <detail/constants.hpp>
+#include <detail/default_case.hpp>
+#include <detail/implementations/cell_impl.hpp>
+#include <detail/implementations/workbook_impl.hpp>
+#include <detail/implementations/worksheet_impl.hpp>
+#include <detail/serialization/excel_thumbnail.hpp>
+#include <detail/serialization/open_stream.hpp>
+#include <detail/serialization/vector_streambuf.hpp>
+#include <detail/serialization/xlsx_consumer.hpp>
+#include <detail/serialization/xlsx_producer.hpp>
 
 namespace {
 
 using xlnt::detail::open_stream;
 
-template<typename T>
+template <typename T>
 std::vector<T> keys(const std::vector<std::pair<T, xlnt::variant>> &container)
 {
     auto result = std::vector<T>();
@@ -80,7 +80,7 @@ std::vector<T> keys(const std::vector<std::pair<T, xlnt::variant>> &container)
     return result;
 }
 
-template<typename T>
+template <typename T>
 bool contains(const std::vector<std::pair<T, xlnt::variant>> &container, const T key)
 {
     for (const auto &iter : container)
@@ -451,29 +451,29 @@ workbook workbook::empty()
     stylesheet.parent = &wb;
 
     auto default_border = border()
-        .side(border_side::bottom, border::border_property())
+                              .side(border_side::bottom, border::border_property())
-        .side(border_side::top, border::border_property())
+                              .side(border_side::top, border::border_property())
-        .side(border_side::start, border::border_property())
+                              .side(border_side::start, border::border_property())
-        .side(border_side::end, border::border_property())
+                              .side(border_side::end, border::border_property())
-        .side(border_side::diagonal, border::border_property());
+                              .side(border_side::diagonal, border::border_property());
     wb.d_->stylesheet_.get().borders.push_back(default_border);
 
     auto default_fill = fill(pattern_fill()
-        .type(pattern_fill_type::none));
+                                 .type(pattern_fill_type::none));
     stylesheet.fills.push_back(default_fill);
     auto gray125_fill = pattern_fill()
-        .type(pattern_fill_type::gray125);
+                            .type(pattern_fill_type::gray125);
     stylesheet.fills.push_back(gray125_fill);
 
     auto default_font = font()
-        .name("Calibri")
+                            .name("Calibri")
-        .size(12)
+                            .size(12)
-        .scheme("minor")
+                            .scheme("minor")
-        .family(2)
+                            .family(2)
-        .color(theme_color(1));
+                            .color(theme_color(1));
     stylesheet.fonts.push_back(default_font);
 
-	wb.create_builtin_style(0)
+    wb.create_builtin_style(0)
         .border(default_border)
         .fill(default_fill)
         .font(default_font)
@@ -550,7 +550,7 @@ void workbook::register_package_part(relationship_type type)
 void workbook::register_workbook_part(relationship_type type)
 {
     auto wb_rel = manifest().relationship(path("/"), relationship_type::office_document);
-    auto wb_path = manifest().canonicalize({ wb_rel });
+    auto wb_path = manifest().canonicalize({wb_rel});
 
     if (!manifest().has_relationship(wb_path, type))
     {
@@ -738,22 +738,35 @@ worksheet workbook::create_sheet()
     std::string title = "Sheet1";
     int index = 1;
 
+    // make a unique sheet name. Sheet<1...n>
     while (contains(title))
     {
         title = "Sheet" + std::to_string(++index);
     }
-
+    // unique sheet id
     size_t sheet_id = 1;
     for (const auto ws : *this)
     {
         sheet_id = std::max(sheet_id, ws.id() + 1);
     }
-    std::string sheet_filename = "sheet" + std::to_string(sheet_id) + ".xml";
-
     d_->worksheets_.push_back(detail::worksheet_impl(this, sheet_id, title));
-
+    // unique sheet file name
     auto workbook_rel = d_->manifest_.relationship(path("/"), relationship_type::office_document);
-    uri relative_sheet_uri(path("worksheets").append(sheet_filename).string());
+    auto workbook_files = d_->manifest_.relationships(workbook_rel.target().path());
+    auto rel_vec_contains = [&workbook_files](const xlnt::path &new_file_id) {
+        return workbook_files.end() != std::find_if(workbook_files.begin(), workbook_files.end(), [&new_file_id](const xlnt::relationship &rel) {
+            return rel.target().path() == new_file_id;
+        });
+    };
+
+    size_t file_id = sheet_id;
+    xlnt::path sheet_relative_path;
+    do
+    {
+        sheet_relative_path = path("worksheets").append("sheet" + std::to_string(file_id++) + ".xml");
+    } while (rel_vec_contains(sheet_relative_path));
+
+    uri relative_sheet_uri(sheet_relative_path.string());
     auto absolute_sheet_path = path("/xl").append(relative_sheet_uri.path());
     d_->manifest_.register_override_type(
         absolute_sheet_path, "application/vnd.openxmlformats-officedocument.spreadsheetml.worksheet+xml");
@@ -1025,7 +1038,8 @@ void workbook::remove_sheet(worksheet ws)
     for (auto &title_rel_id_pair : d_->sheet_title_rel_id_map_)
     {
         title_rel_id_pair.second = rel_id_map.count(title_rel_id_pair.second) > 0
-            ? rel_id_map[title_rel_id_pair.second] : title_rel_id_pair.second;
+            ? rel_id_map[title_rel_id_pair.second]
+            : title_rel_id_pair.second;
     }
 
     update_sheet_properties();
@@ -1390,7 +1404,7 @@ style workbook::create_style(const std::string &name)
 
 style workbook::create_builtin_style(const std::size_t builtin_id)
 {
-	return d_->stylesheet_.get().create_builtin_style(builtin_id);
+    return d_->stylesheet_.get().create_builtin_style(builtin_id);
 }
 
 style workbook::style(const std::string &name)
@@ -1563,69 +1577,83 @@ void workbook::update_sheet_properties()
     }
 }
 
-void workbook::reorder_relationships()
+namespace {
+// true if a sheet index is != worksheet relationship index
+bool needs_reorder(const std::unordered_map<std::string, std::string> &title_to_rels,
+    const std::vector<std::string> &titles,
+    std::vector<std::string> &relation_ids)
 {
-    const auto wb_rel = manifest().relationship(path("/"), relationship_type::office_document);
+    bool all_match = true;
-    const auto wb_path = wb_rel.target().path();
+    for (std::size_t title_index = 0; title_index < titles.size(); ++title_index)
-    const auto relationships = manifest().relationships(wb_path);
-    std::unordered_map<std::string, relationship> rel_map;
-    const auto titles = sheet_titles();
-    const auto title_rel_id_map = d_->sheet_title_rel_id_map_;
-    bool needs_reorder = false;
-
-    for (const auto &rel : relationships)
     {
-        rel_map[rel.id()] = rel;
+        const auto &rel = title_to_rels.at(titles[title_index]);
-
+        relation_ids.push_back(rel);
-        if (rel.type() == relationship_type::worksheet)
+        const auto expected_rel_id = "rId" + std::to_string(title_index + 1);
+        if (rel != expected_rel_id)
         {
-            for (auto title_index = std::size_t(0); title_index < titles.size(); ++title_index)
+            all_match = false;
-            {
-                const auto title = titles[title_index];
-
-                if (title_rel_id_map.at(title) == rel.id())
-                {
-                    const auto expected_rel_id = "rId" + std::to_string(title_index + 1);
-
-                    if (expected_rel_id != rel.id())
-                    {
-                        needs_reorder = true;
-                    }
-
-                    break;
-                }
-            }
         }
     }
+    return !all_match; // if all are as expected, reorder not required
+};
 
-    if (!needs_reorder)
+struct rel_id_sorter
+{
+    // true if lhs < rhs
+    bool operator()(const xlnt::relationship &lhs, const xlnt::relationship &rhs)
+    {
+        // format is rTd<decimal number 1..n>
+        if (lhs.id().size() < rhs.id().size()) // a number with more digits will be larger
+        {
+            return true;
+        }
+        return lhs.id() < rhs.id();
+    }
+};
+} // namespace
+
+void workbook::reorder_relationships()
+{
+    const auto titles = sheet_titles();
+    // the relation ID corresponding to the title at the same index is copied into here
+    std::vector<std::string> worksheet_rel_ids;
+    worksheet_rel_ids.reserve(titles.size());
+    if (!needs_reorder(d_->sheet_title_rel_id_map_, titles, worksheet_rel_ids))
     {
         return;
     }
-
+    // copy of existing relations
-    for (const auto &rel : relationships)
+    const auto wb_rel_target = manifest().relationship(path("/"), relationship_type::office_document).target();
+    auto rel_copy = manifest().relationships(wb_rel_target.path());
+    std::sort(rel_copy.begin(), rel_copy.end(), rel_id_sorter{});
+    // clear existing relations
+    for (const auto &rel : rel_copy)
     {
-        manifest().unregister_relationship(uri(wb_path.string()), rel.id());
+        manifest().unregister_relationship(wb_rel_target, rel.id());
     }
-
+    // create new relations
-    for (auto index = std::size_t(0); index < rel_map.size(); ++index)
+    std::size_t index = 0;
+    auto new_id = [&index]() { return "rId" + std::to_string(++index); }; // ids start from 1
+    // worksheets first
+    while (index < worksheet_rel_ids.size())
     {
-        auto rel_id = "rId" + std::to_string(index + 1);
+        auto rel_it = std::find_if(rel_copy.begin(), rel_copy.end(),
-        auto old_rel_id = std::string();
+            [&](const relationship &rel) { return rel.id() == worksheet_rel_ids[index]; });
 
-        if (index < titles.size())
+        std::string rel_id = new_id();
+        d_->sheet_title_rel_id_map_.at(titles[index - 1]) = rel_id; // update title -> relation mapping
+        manifest().register_relationship(relationship(rel_id, rel_it->type(),
+            rel_it->source(), rel_it->target(), rel_it->target_mode()));
+    }
+    // then all the other relations in the same order they started (just new indices)
+    for (const auto &old_rel : rel_copy)
+    {
+        if (old_rel.type() == relationship_type::worksheet)
         {
-            auto title = titles[index];
+            continue;
-            old_rel_id = title_rel_id_map.at(title);
-            d_->sheet_title_rel_id_map_[title] = rel_id;
-        } else {
-            old_rel_id = "rId" + std::to_string(index - titles.size() + 2);
         }
-
+        manifest().register_relationship(relationship(new_id(), old_rel.type(),
-        auto old_rel = rel_map[old_rel_id];
+            old_rel.source(), old_rel.target(), old_rel.target_mode()));
-        auto new_rel = relationship(rel_id, old_rel.type(),
-            old_rel.source(), old_rel.target(), old_rel.target_mode());
-        manifest().register_relationship(new_rel);
     }
 }
 

source/worksheet/phonetic_pr.cpp

@@ -25,18 +25,26 @@
 #include <array>
 namespace {
 // Order of elements defined by phonetic_pr::Type enum
-const std::array<std::string, 4> Types{
+const std::array<std::string, 4> &Types()
-    "fullwidthKatakana",
+{
-    "halfwidthKatakana",
+    static const std::array<std::string, 4> types{
-    "Hiragana",
+        std::string("fullwidthKatakana"),
-    "noConversion"};
+        std::string("halfwidthKatakana"),
+        std::string("Hiragana"),
+        std::string("noConversion")};
+    return types;
+}
 
 // Order of elements defined by phonetic_pr::alignment enum
-const std::array<std::string, 4> alignments{
+const std::array<std::string, 4> &Alignments()
-    "Center",
+{
-    "Distributed",
+    static const std::array<std::string, 4> alignments{
-    "Left",
+        std::string("Center"),
-    "NoControl"};
+        std::string("Distributed"),
+        std::string("Left"),
+        std::string("NoControl")};
+    return alignments;
+}
 
 } // namespace
 
@@ -44,7 +52,10 @@ namespace xlnt {
 /// <summary>
 /// out of line initialiser for static const member
 /// </summary>
-const std::string phonetic_pr::Serialised_ID = "phoneticPr";
+std::string phonetic_pr::Serialised_ID()
+{
+    return "phoneticPr";
+}
 
 phonetic_pr::phonetic_pr(font_id_t font)
     : font_id_(font)
@@ -53,7 +64,7 @@ phonetic_pr::phonetic_pr(font_id_t font)
 
 void phonetic_pr::serialise(std::ostream &output_stream) const
 {
-    output_stream << '<' << Serialised_ID << R"( fontID=")" << std::to_string(font_id_) << '"';
+    output_stream << '<' << Serialised_ID() << R"( fontID=")" << std::to_string(font_id_) << '"';
     if (has_type())
     {
         output_stream << R"( type=")" << type_as_string(type_.get()) << '"';
@@ -108,14 +119,14 @@ void phonetic_pr::alignment(align align)
 // serialisation
 const std::string &phonetic_pr::type_as_string(phonetic_pr::phonetic_type type)
 {
-    return Types[static_cast<int>(type)];
+    return Types()[static_cast<std::size_t>(type)];
 }
 
 phonetic_pr::phonetic_type phonetic_pr::type_from_string(const std::string &str)
 {
-    for (std::size_t i = 0; i < Types.size(); ++i)
+    for (std::size_t i = 0; i < Types().size(); ++i)
     {
-        if (str == Types[i])
+        if (str == Types()[i])
         {
             return static_cast<phonetic_type>(i);
         }
@@ -125,14 +136,14 @@ phonetic_pr::phonetic_type phonetic_pr::type_from_string(const std::string &str)
 
 const std::string &phonetic_pr::alignment_as_string(align type)
 {
-    return alignments[static_cast<int>(type)];
+    return Alignments()[static_cast<std::size_t>(type)];
 }
 
 phonetic_pr::align phonetic_pr::alignment_from_string(const std::string &str)
 {
-    for (std::size_t i = 0; i < alignments.size(); ++i)
+    for (std::size_t i = 0; i < Alignments().size(); ++i)
     {
-        if (str == alignments[i])
+        if (str == Alignments()[i])
         {
             return static_cast<align>(i);
         }

source/worksheet/range.cpp

@@ -31,7 +31,7 @@
 
 namespace xlnt {
 
-range::range(worksheet ws, const range_reference &reference, major_order order, bool skip_null)
+range::range(class worksheet ws, const range_reference &reference, major_order order, bool skip_null)
     : ws_(ws),
       ref_(reference),
       order_(order),
@@ -69,6 +69,11 @@ cell_vector range::operator[](std::size_t index)
     return vector(index);
 }
 
+const cell_vector range::operator[](std::size_t index) const
+{
+    return vector(index);
+}
+
 const worksheet &range::target_worksheet() const
 {
     return ws_;
@@ -112,6 +117,22 @@ cell_vector range::vector(std::size_t vector_index)
     return cell_vector(ws_, cursor, ref_, order_, skip_null_, false);
 }
 
+const cell_vector range::vector(std::size_t vector_index) const
+{
+    auto cursor = ref_.top_left();
+
+    if (order_ == major_order::row)
+    {
+        cursor.row(cursor.row() + static_cast<row_t>(vector_index));
+    }
+    else
+    {
+        cursor.column_index(cursor.column_index() + static_cast<column_t::index_t>(vector_index));
+    }
+
+    return cell_vector(ws_, cursor, ref_, order_, skip_null_, false);
+}
+
 bool range::contains(const cell_reference &ref)
 {
     return ref_.top_left().column_index() <= ref.column_index()
@@ -188,6 +209,11 @@ cell range::cell(const cell_reference &ref)
     return (*this)[ref.row() - 1][ref.column().index - 1];
 }
 
+const cell range::cell(const cell_reference &ref) const
+{
+    return (*this)[ref.row() - 1][ref.column().index - 1];
+}
+
 cell_vector range::front()
 {
     return *begin();

source/worksheet/worksheet.cpp

@@ -98,7 +98,7 @@ void worksheet::create_named_range(const std::string &name, const range_referenc
             throw invalid_parameter(); //("named range name must be outside the range A1-XFD1048576");
         }
     }
-    catch (xlnt::invalid_cell_reference)
+    catch (xlnt::invalid_cell_reference&)
     {
         // name is not a valid reference, that's good
     }

tests/CMakeLists.txt

@@ -1,9 +1,9 @@
 cmake_minimum_required(VERSION 3.1)
 project(xlnt.test)
 
-# Require C++11 compiler
+set(CMAKE_CXX_STANDARD ${XLNT_CXX_LANG})
-set(CMAKE_CXX_STANDARD 11)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CXX_EXTENSIONS OFF)
 
 if(NOT COMBINED_PROJECT)
   add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/../source ${CMAKE_CURRENT_BINARY_DIR}/source)
@@ -48,6 +48,8 @@ target_include_directories(xlnt.test
 
 set(XLNT_TEST_DATA_DIR ${CMAKE_CURRENT_SOURCE_DIR}/data)
 target_compile_definitions(xlnt.test PRIVATE XLNT_TEST_DATA_DIR=${XLNT_TEST_DATA_DIR})
+# requires cmake 3.8+
+#target_compile_features(xlnt.test PRIVATE cxx_std_${XLNT_CXX_LANG})
 
 if(MSVC)
   # bigobj because there are so many headers in one source file

tests/cell/cell_test_suite.cpp

@@ -29,18 +29,20 @@
 #include <xlnt/cell/cell.hpp>
 #include <xlnt/cell/comment.hpp>
 #include <xlnt/cell/hyperlink.hpp>
-#include <xlnt/styles/number_format.hpp>
 #include <xlnt/styles/alignment.hpp>
 #include <xlnt/styles/border.hpp>
 #include <xlnt/styles/fill.hpp>
 #include <xlnt/styles/format.hpp>
+#include <xlnt/styles/number_format.hpp>
 #include <xlnt/styles/protection.hpp>
 #include <xlnt/styles/style.hpp>
-#include <xlnt/worksheet/worksheet.hpp>
 #include <xlnt/utils/date.hpp>
 #include <xlnt/utils/datetime.hpp>
 #include <xlnt/utils/time.hpp>
 #include <xlnt/utils/timedelta.hpp>
+#include <xlnt/workbook/workbook.hpp>
+#include <xlnt/worksheet/range.hpp>
+#include <xlnt/worksheet/worksheet.hpp>
 
 class cell_test_suite : public test_suite
 {
@@ -79,6 +81,7 @@ public:
         register_test(test_anchor);
         register_test(test_hyperlink);
         register_test(test_comment);
+        register_test(test_copy_and_compare);
     }
 
 private:
@@ -251,11 +254,27 @@ private:
         xlnt::workbook wb;
         auto ws = wb.active_sheet();
         auto cell = ws.cell(xlnt::cell_reference(1, 1));
+        // error string can't be empty
+        xlnt_assert_throws(cell.error(""), xlnt::exception);
+        // error string has to have a leading '#'
+        xlnt_assert_throws(cell.error("not an error"), xlnt::exception);
 
         for (auto error_code : xlnt::cell::error_codes())
         {
+            // error type from the string format
             cell.value(error_code.first, true);
             xlnt_assert(cell.data_type() == xlnt::cell::type::error);
+            std::string error;
+            xlnt_assert_throws_nothing(error = cell.error());
+            xlnt_assert_equals(error, error_code.first);
+            // clearing the value clears the error state
+            cell.clear_value();
+            xlnt_assert_throws(cell.error(), xlnt::exception);
+            // can explicitly set the error
+            xlnt_assert_throws_nothing(cell.error(error_code.first));
+            std::string error2;
+            xlnt_assert_throws_nothing(error2 = cell.error());
+            xlnt_assert_equals(error2, error_code.first);
         }
     }
 
@@ -665,13 +684,81 @@ private:
     {
         xlnt::workbook wb;
         auto cell = wb.active_sheet().cell("A1");
+
         xlnt_assert(!cell.has_hyperlink());
         xlnt_assert_throws(cell.hyperlink(), xlnt::invalid_attribute);
         xlnt_assert_throws(cell.hyperlink("notaurl"), xlnt::invalid_parameter);
         xlnt_assert_throws(cell.hyperlink(""), xlnt::invalid_parameter);
-        cell.hyperlink("http://example.com");
+        // link without optional display
+        const std::string link1("http://example.com");
+        cell.hyperlink(link1);
         xlnt_assert(cell.has_hyperlink());
-        xlnt_assert_equals(cell.hyperlink().relationship().target().to_string(), "http://example.com");
+        xlnt_assert(cell.hyperlink().external());
+        xlnt_assert_equals(cell.hyperlink().url(), link1);
+        xlnt_assert_equals(cell.hyperlink().relationship().target().to_string(), link1);
+        xlnt_assert_equals(cell.hyperlink().display(), link1);
+        cell.clear_value();
+        // link with display
+        const std::string link2("http://example2.com");
+        const std::string display_txt("notaurl");
+        cell.hyperlink(link2, display_txt);
+        xlnt_assert(cell.has_hyperlink());
+        xlnt_assert(cell.hyperlink().external());
+        xlnt_assert_equals(cell.hyperlink().url(), link2);
+        xlnt_assert_equals(cell.hyperlink().relationship().target().to_string(), link2);
+        xlnt_assert_equals(cell.hyperlink().display(), display_txt);
+        // value
+        int cell_test_val = 123;
+        cell.value(cell_test_val);
+        std::string cell_value_str = std::to_string(cell_test_val);
+        cell.hyperlink(link2, display_txt);
+        xlnt_assert_equals(cell.value<int>(), 123);
+        xlnt_assert_equals(cell.hyperlink().display(), cell_value_str); // display text ignored
+        cell.clear_value();
+        // cell overload without display
+        const std::string cell_target_str("A2");
+        auto cell_target = wb.active_sheet().cell(cell_target_str);
+        std::string link3 = wb.active_sheet().title() + '!' + cell_target_str; // Sheet1!A2
+        cell.hyperlink(cell_target);
+        xlnt_assert(cell.has_hyperlink());
+        xlnt_assert(!cell.hyperlink().external());
+        xlnt_assert_equals(cell.hyperlink().target_range(), link3);
+        xlnt_assert_equals(cell.hyperlink().display(), link3);
+        cell.clear_value();
+        // cell overload with display
+        cell.hyperlink(cell_target, display_txt);
+        xlnt_assert(cell.has_hyperlink());
+        xlnt_assert(!cell.hyperlink().external());
+        xlnt_assert_equals(cell.hyperlink().target_range(), link3);
+        xlnt_assert_equals(cell.hyperlink().display(), display_txt);
+        // value
+        cell.value(cell_test_val);
+        cell.hyperlink(cell_target, display_txt);
+        xlnt_assert_equals(cell.value<int>(), 123);
+        xlnt_assert_equals(cell.hyperlink().display(), cell_value_str); // display text ignored
+        cell.clear_value();
+        // range overload without display
+        const std::string range_target_str("A2:A5");
+        xlnt::range range_target(wb.active_sheet(), xlnt::range_reference(range_target_str));
+        std::string link4 = wb.active_sheet().title() + '!' + range_target_str; // Sheet1!A2:A5
+        cell.hyperlink(range_target);
+        xlnt_assert(cell.has_hyperlink());
+        xlnt_assert(!cell.hyperlink().external());
+        xlnt_assert_equals(cell.hyperlink().target_range(), link4);
+        xlnt_assert_equals(cell.hyperlink().display(), link4);
+        cell.clear_value();
+        // range overload with display
+        cell.hyperlink(range_target, display_txt);
+        xlnt_assert(cell.has_hyperlink());
+        xlnt_assert(!cell.hyperlink().external());
+        xlnt_assert_equals(cell.hyperlink().target_range(), link4);
+        xlnt_assert_equals(cell.hyperlink().display(), display_txt);
+        // value
+        cell.value(cell_test_val);
+        cell.hyperlink(range_target, display_txt);
+        xlnt_assert_equals(cell.value<int>(), 123);
+        xlnt_assert_equals(cell.hyperlink().display(), cell_value_str); // display text ignored
+        cell.clear_value();
     }
 
     void test_comment()
@@ -688,6 +775,31 @@ private:
         xlnt_assert(!cell.has_comment());
         xlnt_assert_throws(cell.comment(), xlnt::exception);
     }
+
+    void test_copy_and_compare()
+    {
+        xlnt::workbook wb;
+        auto ws = wb.active_sheet();
+        auto cell1 = ws.cell("A1");
+        auto cell2 = ws.cell("A2");
+
+        xlnt_assert_equals(cell1, cell1);
+        xlnt_assert_equals(cell2, cell2);
+        xlnt_assert_differs(cell1, cell2);
+        xlnt_assert_differs(cell2, cell1);
+        // nullptr equality
+        xlnt_assert(nullptr == cell1);
+        xlnt_assert(cell1 == nullptr);
+        cell1.value("test");
+        xlnt_assert(!(nullptr == cell1));
+        xlnt_assert(!(cell1 == nullptr));
+        // copy
+        xlnt::cell cell3(cell1);
+        xlnt_assert_equals(cell1, cell3);
+        // assign
+        cell3 = cell2;
+        xlnt_assert_equals(cell2, cell3);
+    }
 };
 
 static cell_test_suite x{};

tests/cell/rich_text_test_suite.cpp

@@ -35,6 +35,7 @@ public:
     rich_text_test_suite()
     {
         register_test(test_operators);
+        register_test(test_runs);
     }
 
     void test_operators()
@@ -100,5 +101,19 @@ public:
         text_family_differs.add_run(run_family_differs);
         xlnt_assert_differs(text_formatted, text_family_differs);
     }
+
+    void test_runs()
+    {
+        xlnt::rich_text rt;
+        xlnt_assert(rt.runs().empty());
+        std::vector<xlnt::rich_text_run> test_runs{xlnt::rich_text_run{"1_abc_test_123"}, xlnt::rich_text_run{"2_abc_test_123"}, xlnt::rich_text_run{"3_abc_test_123"}};
+        // just one
+        rt.add_run(test_runs[0]);
+        xlnt_assert_equals(1, rt.runs().size());
+        xlnt_assert_equals(test_runs[0], rt.runs()[0]);
+        // whole set
+        rt.runs(test_runs);
+        xlnt_assert_equals(test_runs, rt.runs());
+    }
 };
 static rich_text_test_suite x{};

tests/helpers/xml_helper.hpp

@@ -3,17 +3,17 @@
 #include <sstream>
 #include <unordered_set>
 
+#include <xlnt/packaging/manifest.hpp>
+#include <xlnt/workbook/workbook.hpp>
 #include <detail/external/include_libstudxml.hpp>
 #include <detail/serialization/vector_streambuf.hpp>
 #include <detail/serialization/zstream.hpp>
-#include <xlnt/packaging/manifest.hpp>
-#include <xlnt/workbook/workbook.hpp>
 
 class xml_helper
 {
 public:
     static bool compare_files(const std::string &left,
-		const std::string &right, const std::string &content_type)
+        const std::string &right, const std::string &content_type)
     {
         // content types are stored in unordered maps, too complicated to compare
         if (content_type == "[Content_Types].xml")
@@ -34,7 +34,7 @@ public:
         }
 
         auto is_xml = (content_type.substr(0, 12) == "application/"
-            && content_type.substr(content_type.size() - 4) == "+xml")
+                          && content_type.substr(content_type.size() - 4) == "+xml")
             || content_type == "application/xml"
             || content_type == "[Content_Types].xml"
             || content_type == "application/vnd.openxmlformats-officedocument.vmlDrawing";
@@ -63,8 +63,7 @@ public:
         bool difference = false;
         auto right_iter = right_parser.begin();
 
-        auto is_whitespace = [](const std::string &v)
+        auto is_whitespace = [](const std::string &v) {
-        {
             return v.find_first_not_of("\n\r\t ") == std::string::npos;
         };
 
@@ -198,26 +197,26 @@ public:
             ++right_iter;
         }
 
-		if (difference && !suppress_debug_info)
+        if (difference && !suppress_debug_info)
-		{
+        {
-			std::cout << "documents don't match" << std::endl;
+            std::cout << "documents don't match" << std::endl;
 
-			std::cout << "left:" << std::endl;
+            std::cout << "left:" << std::endl;
             for (auto c : left)
             {
                 std::cout << c << std::flush;
             }
-			std::cout << std::endl;
+            std::cout << std::endl;
 
-			std::cout << "right:" << std::endl;
+            std::cout << "right:" << std::endl;
             for (auto c : right)
             {
                 std::cout << c << std::flush;
             }
-			std::cout << std::endl;
+            std::cout << std::endl;
-		}
+        }
 
-		return !difference;
+        return !difference;
     }
 
     static bool compare_relationships(const xlnt::manifest &left,
@@ -271,27 +270,26 @@ public:
         return true;
     }
 
-	static bool xlsx_archives_match(const std::vector<std::uint8_t> &left,
+    static bool xlsx_archives_match(const std::vector<std::uint8_t> &left,
         const std::vector<std::uint8_t> &right)
-	{
+    {
         xlnt::detail::vector_istreambuf left_buffer(left);
         std::istream left_stream(&left_buffer);
         xlnt::detail::izstream left_archive(left_stream);
 
-		const auto left_info = left_archive.files();
+        const auto left_info = left_archive.files();
 
         xlnt::detail::vector_istreambuf right_buffer(right);
         std::istream right_stream(&right_buffer);
         xlnt::detail::izstream right_archive(right_stream);
 
-		const auto right_info = right_archive.files();
+        const auto right_info = right_archive.files();
 
         auto difference_is_missing_calc_chain = false;
 
         if (std::abs(int(left_info.size()) - int(right_info.size())) == 1)
         {
-            auto is_calc_chain = [](const xlnt::path &p)
+            auto is_calc_chain = [](const xlnt::path &p) {
-            {
                 return p.filename() == "calcChain.xml";
             };
 
@@ -306,7 +304,7 @@ public:
             }
         }
 
-		if (left_info.size() != right_info.size() && ! difference_is_missing_calc_chain)
+        if (left_info.size() != right_info.size() && !difference_is_missing_calc_chain)
         {
             std::cout << "left has a different number of files than right" << std::endl;
 
@@ -338,12 +336,41 @@ public:
 
         if (!compare_relationships(left_manifest, right_manifest))
         {
+            std::cout << "relationship mismatch\n"
+                      << "Left:\n";
+            for (const auto &part : left_manifest.parts())
+            {
+                std::cout << "-part: " << part.string() << '\n';
+                auto rels = left_manifest.relationships(part);
+                for (auto &rel : rels)
+                {
+                    std::cout << rel.id() << ':' 
+                        << static_cast<int>(rel.type()) 
+                        << ':' << static_cast<int>(rel.target_mode()) 
+                        << ':' << rel.source().path().string() 
+                        << ':' << rel.target().path().string() << '\n';
+                }
+            }
+            std::cout << "\nRight:\n";
+            for (const auto &part : right_manifest.parts())
+            {
+                std::cout << "-part: " << part.string() << '\n';
+                auto rels = right_manifest.relationships(part);
+                for (auto &rel : rels)
+                {
+                    std::cout << rel.id() 
+                        << ':' << static_cast<int>(rel.type()) 
+                        << ':' << static_cast<int>(rel.target_mode()) 
+                        << ':' << rel.source().path().string() 
+                        << ':' << rel.target().path().string() << '\n';
+                }
+            }
             return false;
         }
 
-		for (auto left_member : left_info)
+        for (auto left_member : left_info)
-		{
+        {
-			if (!right_archive.has_file(left_member))
+            if (!right_archive.has_file(left_member))
             {
                 if (difference_is_missing_calc_chain)
                 {
@@ -357,32 +384,34 @@ public:
             }
 
             auto left_content_type = left_member.string() == "[Content_Types].xml"
-                ? "[Content_Types].xml" : left_manifest.content_type(left_member);
+                ? "[Content_Types].xml"
+                : left_manifest.content_type(left_member);
             auto right_content_type = left_member.string() == "[Content_Types].xml"
-                ? "[Content_Types].xml" : right_manifest.content_type(left_member);
+                ? "[Content_Types].xml"
+                : right_manifest.content_type(left_member);
 
             if (left_content_type != right_content_type)
             {
                 std::cout << "content types differ: "
-                    << left_member.string()
+                          << left_member.string()
-                    << " "
+                          << " "
-                    << left_content_type
+                          << left_content_type
-                    << " "
+                          << " "
-                    << right_content_type
+                          << right_content_type
-                    << std::endl;
+                          << std::endl;
                 match = false;
                 break;
             }
 
             if (!compare_files(left_archive.read(left_member),
-                right_archive.read(left_member), left_content_type))
+                    right_archive.read(left_member), left_content_type))
-			{
+            {
-				std::cout << left_member.string() << std::endl;
+                std::cout << left_member.string() << std::endl;
                 match = false;
                 break;
-			}
+            }
-		}
+        }
 
-		return match;
+        return match;
-	}
+    }
 };

tests/styles/color_test_suite.cpp

@@ -38,37 +38,39 @@ public:
 
     void test_known_colors()
     {
-        const std::vector<std::pair<xlnt::color, std::string>> known_colors
+        const std::vector<std::pair<xlnt::color, std::string>> known_colors{
-        {
+            {xlnt::color::black(), "FF000000"},
-            { xlnt::color::black(), "FF000000" },
+            {xlnt::color::white(), "FFFFFFFF"},
-            { xlnt::color::white(), "FFFFFFFF" },
+            {xlnt::color::red(), "FFFF0000"},
-            { xlnt::color::red(), "FFFF0000" },
+            {xlnt::color::darkred(), "FF8B0000"},
-            { xlnt::color::darkred(), "FF8B0000" },
+            {xlnt::color::blue(), "FF0000FF"},
-            { xlnt::color::blue(), "FF0000FF" },
+            {xlnt::color::darkblue(), "FF00008B"},
-            { xlnt::color::darkblue(), "FF00008B" },
+            {xlnt::color::green(), "FF00FF00"},
-            { xlnt::color::green(), "FF00FF00" },
+            {xlnt::color::darkgreen(), "FF008B00"},
-            { xlnt::color::darkgreen(), "FF008B00" },
+            {xlnt::color::yellow(), "FFFFFF00"},
-            { xlnt::color::yellow(), "FFFFFF00" },
+            {xlnt::color::darkyellow(), "FFCCCC00"}};
-            { xlnt::color::darkyellow(), "FFCCCC00" }
+
-        };
-        
         for (auto pair : known_colors)
         {
             xlnt_assert_equals(pair.first.rgb().hex_string(), pair.second);
         }
     }
-    
+
     void test_non_rgb_colors()
     {
-		xlnt::color indexed = xlnt::indexed_color(1);
+        xlnt::color indexed = xlnt::indexed_color(1);
-		xlnt_assert(!indexed.auto_());
+        xlnt_assert(!indexed.auto_());
         xlnt_assert_equals(indexed.indexed().index(), 1);
+        indexed.indexed().index(2);
+        xlnt_assert_equals(indexed.indexed().index(), 2);
         xlnt_assert_throws(indexed.theme(), xlnt::invalid_attribute);
         xlnt_assert_throws(indexed.rgb(), xlnt::invalid_attribute);
 
-		xlnt::color theme = xlnt::theme_color(3);
+        xlnt::color theme = xlnt::theme_color(3);
-		xlnt_assert(!theme.auto_());
+        xlnt_assert(!theme.auto_());
         xlnt_assert_equals(theme.theme().index(), 3);
+        theme.theme().index(4);
+        xlnt_assert_equals(theme.theme().index(), 4);
         xlnt_assert_throws(theme.indexed(), xlnt::invalid_attribute);
         xlnt_assert_throws(theme.rgb(), xlnt::invalid_attribute);
     }

tests/styles/conditional_format_test_suite.cpp

@@ -0,0 +1,64 @@
+// Copyright (c) 2014-2018 Thomas Fussell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+
+#include <xlnt/styles/conditional_format.hpp>
+#include <helpers/test_suite.hpp>
+#include <xlnt/xlnt.hpp>
+
+class conditional_format_test_suite : public test_suite
+{
+public:
+    conditional_format_test_suite()
+    {
+        register_test(test_all);
+    }
+
+    void test_all()
+    {
+        xlnt::workbook wb;
+        auto ws = wb.active_sheet();
+        auto format = ws.conditional_format(xlnt::range_reference("A1:A10"), xlnt::condition::text_contains("test"));
+        xlnt_assert(!format.has_border());
+        xlnt_assert(!format.has_fill());
+        xlnt_assert(!format.has_font());
+        // set border
+        auto border = xlnt::border().diagonal(xlnt::diagonal_direction::both);
+        format.border(border);
+        xlnt_assert(format.has_border());
+        xlnt_assert_equals(format.border(), border);
+        // set fill
+        auto fill = xlnt::fill(xlnt::gradient_fill().type(xlnt::gradient_fill_type::path));
+        format.fill(fill);
+        xlnt_assert(format.has_fill());
+        xlnt_assert_equals(format.fill(), fill);
+        // set font
+        auto font = xlnt::font().color(xlnt::color::darkblue());
+        format.font(font);
+        xlnt_assert(format.has_font());
+        xlnt_assert_equals(format.font(), font);
+        // copy ctor
+        auto format_copy(format);
+        xlnt_assert_equals(format, format_copy);
+    }
+};
+static conditional_format_test_suite x;

tests/styles/protection_test_suite.cpp

@@ -0,0 +1,55 @@
+// Copyright (c) 2014-2018 Thomas Fussell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+
+#include <xlnt/styles/protection.hpp>
+#include <helpers/test_suite.hpp>
+#include <xlnt/xlnt.hpp>
+
+class protection_test_suite : public test_suite
+{
+public:
+    protection_test_suite()
+    {
+        register_test(test_all);
+    }
+
+    void test_all()
+    {
+        auto prot = xlnt::protection::unlocked_and_visible();
+        xlnt_assert(!prot.hidden());
+        xlnt_assert(!prot.locked());
+
+        prot = xlnt::protection::locked_and_visible();
+        xlnt_assert(!prot.hidden());
+        xlnt_assert(prot.locked());
+
+        prot = xlnt::protection::unlocked_and_hidden();
+        xlnt_assert(prot.hidden());
+        xlnt_assert(!prot.locked());
+
+        prot = xlnt::protection::locked_and_hidden();
+        xlnt_assert(prot.hidden());
+        xlnt_assert(prot.locked());
+    }
+};
+static protection_test_suite x;

tests/styles/style_test_suite.cpp

@@ -0,0 +1,66 @@
+// Copyright (c) 2014-2018 Thomas Fussell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+
+#include <xlnt/styles/style.hpp>
+#include <xlnt/xlnt.hpp>
+#include <helpers/test_suite.hpp>
+
+class style_test_suite : public test_suite
+{
+public:
+    style_test_suite()
+    {
+        register_test(test_all);
+    }
+
+    void test_all()
+    {
+        xlnt::workbook wb;
+        auto ws = wb.active_sheet();
+        auto test_style = wb.create_style("test_style");
+        test_style.number_format(xlnt::number_format::date_ddmmyyyy());
+
+        auto copy_style(test_style);
+        xlnt_assert_equals(test_style, copy_style);
+
+        // number format
+        xlnt_assert_equals(copy_style.name(), "test_style");
+        xlnt_assert_equals(copy_style.number_format(), xlnt::number_format::date_ddmmyyyy());
+        //xlnt_assert(!copy_style.number_format_applied()); // this doesn't seem to have sensible behaviour?
+        copy_style.number_format(xlnt::number_format::date_datetime(), true); // true applied param
+        xlnt_assert_equals(copy_style.number_format(), xlnt::number_format::date_datetime());
+        xlnt_assert(copy_style.number_format_applied());
+        copy_style.number_format(xlnt::number_format::date_dmminus(), false); // false applied param
+        xlnt_assert_equals(copy_style.number_format(), xlnt::number_format::date_dmminus());
+        xlnt_assert(!copy_style.number_format_applied());
+
+        xlnt_assert(!copy_style.pivot_button());
+        copy_style.pivot_button(true);
+        xlnt_assert(copy_style.pivot_button());
+
+        xlnt_assert(!copy_style.quote_prefix());
+        copy_style.quote_prefix(true);
+        xlnt_assert(copy_style.quote_prefix());
+    }
+};
+static style_test_suite x;

tests/utils/optional_tests.cpp

@@ -0,0 +1,307 @@
+// Copyright (c) 2014-2018 Thomas Fussell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+
+#include <xlnt/xlnt.hpp>
+#include <xlnt/utils/optional.hpp>
+#include <helpers/test_suite.hpp>
+
+// test helpers
+namespace {
+// increments count when constructed, decrements when destructed
+// use to ensure correct ctor/dtor pairing
+struct alive_count
+{
+    alive_count()
+    {
+        ++count;
+    }
+
+    alive_count(const alive_count &)
+    {
+        ++count;
+    }
+
+    alive_count(alive_count &&)
+    {
+        ++count;
+    }
+
+    ~alive_count()
+    {
+        --count;
+    }
+
+    alive_count &operator=(const alive_count &) = default;
+    alive_count &operator=(alive_count &&) = default;
+
+    static int count;
+};
+int alive_count::count = 0;
+
+// implicitly convertible from int
+struct convertible
+{
+    // implicit construction from int
+    convertible(int i)
+        : val(i)
+    {
+    }
+
+    int val;
+};
+
+// default ctor deleted
+struct no_default
+{
+    no_default() = delete;
+    int i;
+};
+} // namespace
+
+class optional_test_suite : public test_suite
+{
+public:
+    optional_test_suite()
+        : test_suite()
+    {
+        register_test(test_ctor);
+        register_test(test_copy_ctor);
+        register_test(test_move_ctor);
+        register_test(test_copy_assign);
+        register_test(test_move_assign);
+        register_test(test_set_and_get);
+        register_test(test_equality);
+        register_test(test_const);
+    }
+
+    void test_ctor()
+    {
+        // default
+        xlnt::optional<int> opt1;
+        xlnt_assert(!opt1.is_set());
+        // value
+        const int test_val = 3;
+        xlnt::optional<int> opt2(test_val);
+        xlnt_assert(opt2.is_set());
+        xlnt_assert_equals(opt2.get(), test_val);
+        // converting
+        xlnt::optional<convertible> opt3(test_val);
+        xlnt_assert(opt3.is_set());
+        xlnt_assert_equals(opt3.get().val, test_val);
+        // no default ctor
+        xlnt::optional<no_default> no_def_opt;
+    }
+
+    void test_copy_ctor()
+    {
+        { // copy behaviour
+            xlnt::optional<int> opt1;
+            xlnt::optional<int> opt2(opt1);
+            xlnt_assert_equals(opt1, opt2);
+
+            const int test_val = 123;
+            xlnt::optional<int> opt3(test_val);
+            xlnt::optional<int> opt4(opt3);
+            xlnt_assert_equals(opt3, opt4);
+        }
+        { // lifetime checks
+            xlnt::optional<alive_count> opt1(alive_count{});
+            xlnt_assert_equals(1, alive_count::count);
+            {
+                xlnt::optional<alive_count> opt2(opt1);
+                xlnt_assert_equals(2, alive_count::count);
+            }
+            xlnt_assert_equals(1, alive_count::count);
+        }
+        xlnt_assert_equals(0, alive_count::count); // dtor test
+    }
+
+    void test_move_ctor()
+    {
+        { // move behaviour
+            xlnt::optional<int> opt1;
+            xlnt::optional<int> opt2(std::move(opt1));
+            xlnt_assert_equals(opt2, xlnt::optional<int>{}); // can't test against opt1 so use a temporary
+
+            const int test_val = 123;
+            xlnt::optional<int> opt3(test_val);
+            xlnt::optional<int> opt4(std::move(opt3));
+            xlnt_assert(opt4.is_set()); // moved to optional contains the value
+            xlnt_assert_equals(opt4.get(), test_val);
+        }
+        { // lifetime checks
+            xlnt::optional<alive_count> opt1(alive_count{});
+            xlnt_assert_equals(1, alive_count::count);
+            {
+                xlnt::optional<alive_count> opt2(std::move(opt1));
+                xlnt_assert_equals(1, alive_count::count); // opt1 is in a no-value state
+            }
+            xlnt_assert_equals(0, alive_count::count);
+        }
+        xlnt_assert_equals(0, alive_count::count); // dtor test
+    }
+
+    void test_copy_assign()
+    {
+        { // copy assign behaviour
+            xlnt::optional<int> opt1;
+            xlnt::optional<int> opt_assign1; // to actually test assignment, the value needs to be already created. using '=' is not enough
+            opt_assign1 = opt1;
+            xlnt_assert_equals(opt1, opt_assign1);
+
+            const int test_val = 123;
+            xlnt::optional<int> opt2(test_val);
+            xlnt::optional<int> opt_assign2;
+            opt_assign2 = opt2;
+            xlnt_assert_equals(opt2, opt_assign2);
+        }
+        { // lifetime checks
+            xlnt::optional<alive_count> opt1(alive_count{});
+            xlnt_assert_equals(1, alive_count::count);
+            {
+                xlnt::optional<alive_count> opt_assign1;
+                opt_assign1 = opt1;
+                xlnt_assert_equals(2, alive_count::count);
+            }
+            xlnt_assert_equals(1, alive_count::count);
+        }
+        xlnt_assert_equals(0, alive_count::count); // dtor test
+    }
+
+    void test_move_assign()
+    {
+        { // copy assign behaviour
+            xlnt::optional<int> opt1;
+            xlnt::optional<int> opt_assign1; // to actually test assignment, the value needs to be already created. using '=' is not enough
+            opt_assign1 = std::move(opt1);
+            xlnt_assert_equals(opt_assign1, xlnt::optional<int>{}); // can't test against opt1 so use a temporary
+
+            const int test_val = 123;
+            xlnt::optional<int> opt2(test_val);
+            xlnt::optional<int> opt_assign2;
+            opt_assign2 = std::move(opt2);
+            xlnt_assert(opt_assign2.is_set()); // moved to optional contains the value
+            xlnt_assert_equals(opt_assign2.get(), test_val);
+        }
+        { // lifetime checks
+            xlnt::optional<alive_count> opt1(alive_count{});
+            xlnt_assert_equals(1, alive_count::count);
+            {
+                xlnt::optional<alive_count> opt_assign1;
+                opt_assign1 = std::move(opt1);
+                xlnt_assert_equals(1, alive_count::count); // opt1 is in a no-value state
+            }
+            xlnt_assert_equals(0, alive_count::count);
+        }
+        xlnt_assert_equals(0, alive_count::count); // dtor test
+    }
+
+    void test_set_and_get()
+    {
+        {
+            xlnt::optional<int> test_opt;
+            xlnt_assert(!test_opt.is_set());
+            xlnt_assert_throws(test_opt.get(), xlnt::invalid_attribute);
+            // set
+            const int test_val1 = 321;
+            test_opt.set(test_val1);
+            xlnt_assert(test_opt.is_set());
+            xlnt_assert_equals(test_opt.get(), test_val1);
+            // set again
+            const int test_val2 = 123;
+            test_opt.set(test_val2);
+            xlnt_assert(test_opt.is_set());
+            xlnt_assert_equals(test_opt.get(), test_val2);
+            // clear
+            test_opt.clear();
+            xlnt_assert(!test_opt.is_set());
+            xlnt_assert_throws(test_opt.get(), xlnt::invalid_attribute);
+            // set again
+            const int test_val3 = 3;
+            test_opt.set(test_val3);
+            xlnt_assert(test_opt.is_set());
+            xlnt_assert_equals(test_opt.get(), test_val3);
+            // operator= set
+            xlnt::optional<int> test_opt2;
+            test_opt2 = test_val1;
+            xlnt_assert_equals(test_opt2.get(), test_val1);
+        }
+        { // lifetime checks
+            xlnt::optional<alive_count> test_opt;
+            xlnt_assert_equals(0, alive_count::count);
+            test_opt.set(alive_count());
+            xlnt_assert_equals(1, alive_count::count);
+            test_opt.set(alive_count()); // reassignment doesn't somehow skip the dtor
+            xlnt_assert_equals(1, alive_count::count);
+            test_opt.clear();
+            xlnt_assert_equals(0, alive_count::count);
+        }
+    }
+
+    void test_equality()
+    {
+        xlnt::optional<int> test_opt1;
+        xlnt::optional<int> test_opt2;
+        // no value opts compare equal
+        xlnt_assert(test_opt1 == test_opt2);
+        xlnt_assert(!(test_opt1 != test_opt2));
+        xlnt_assert(test_opt2 == test_opt1);
+        xlnt_assert(!(test_opt2 != test_opt1));
+        // value compares false with no value
+        const int test_val = 1;
+        test_opt1.set(test_val);
+        xlnt_assert(test_opt1 != test_opt2);
+        xlnt_assert(!(test_opt1 == test_opt2));
+        xlnt_assert(test_opt2 != test_opt1);
+        xlnt_assert(!(test_opt2 == test_opt1));
+        // value compares false with a different value
+        const int test_val2 = 2;
+        test_opt2.set(test_val2);
+        xlnt_assert(test_opt1 != test_opt2);
+        xlnt_assert(!(test_opt1 == test_opt2));
+        xlnt_assert(test_opt2 != test_opt1);
+        xlnt_assert(!(test_opt2 == test_opt1));
+        // value compares equal with same value
+        test_opt2.set(test_val);
+        xlnt_assert(test_opt1 == test_opt2);
+        xlnt_assert(!(test_opt1 != test_opt2));
+        xlnt_assert(test_opt2 == test_opt1);
+        xlnt_assert(!(test_opt2 != test_opt1));
+    }
+
+    void test_const()
+    {
+        // functions on a const optional
+        const int test_val = 1;
+        const xlnt::optional<int> opt(test_val);
+        xlnt_assert(opt.is_set());
+        xlnt_assert(opt.get() == test_val);
+
+        xlnt::optional<int> opt2(test_val);
+        xlnt_assert(opt == opt2);
+        xlnt_assert(opt2 == opt);
+        xlnt_assert(!(opt != opt2));
+        xlnt_assert(!(opt2 != opt));
+    }
+};
+static optional_test_suite x;

tests/utils/path_test_suite.cpp

@@ -26,6 +26,7 @@
 #include <helpers/path_helper.hpp>
 #include <helpers/temporary_file.hpp>
 #include <helpers/test_suite.hpp>
+#include <xlnt/utils/path.hpp>
 
 class path_test_suite : public test_suite
 {

tests/utils/variant_tests.cpp

@@ -0,0 +1,69 @@
+// Copyright (c) 2014-2018 Thomas Fussell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+
+#include <xlnt/utils/variant.hpp>
+#include <helpers/test_suite.hpp>
+
+class variant_test_suite : public test_suite
+{
+public:
+    variant_test_suite()
+        : test_suite()
+    {
+        register_test(test_null);
+        register_test(test_int32);
+        register_test(test_string);
+    }
+
+    void test_null()
+    {
+        xlnt::variant var_null;
+        xlnt_assert_equals(var_null.value_type(), xlnt::variant::type::null);
+        xlnt_assert(var_null.is(xlnt::variant::type::null));
+    }
+
+    void test_int32()
+    {
+        xlnt::variant var_int(std::int32_t(10));
+        xlnt_assert_equals(var_int.value_type(), xlnt::variant::type::i4);
+        xlnt_assert(var_int.is(xlnt::variant::type::i4));
+        xlnt_assert_throws_nothing(var_int.get<std::int32_t>());
+        xlnt_assert_equals(10, var_int.get<std::int32_t>());
+    }
+
+    void test_string()
+    {
+        xlnt::variant var_str1("test1");
+        xlnt_assert_equals(var_str1.value_type(), xlnt::variant::type::lpstr);
+        xlnt_assert(var_str1.is(xlnt::variant::type::lpstr));
+        xlnt_assert_throws_nothing(var_str1.get<std::string>());
+        xlnt_assert_equals("test1", var_str1.get<std::string>());
+
+        xlnt::variant var_str2(std::string("test2"));
+        xlnt_assert_equals(var_str2.value_type(), xlnt::variant::type::lpstr);
+        xlnt_assert(var_str2.is(xlnt::variant::type::lpstr));
+        xlnt_assert_throws_nothing(var_str2.get<std::string>());
+        xlnt_assert_equals("test2", var_str2.get<std::string>());
+    }
+};
+static variant_test_suite x;

tests/workbook/serialization_test_suite.cpp

@@ -575,7 +575,7 @@ public:
 
         std::vector<std::uint8_t> destination;
         source_workbook.save(destination);
-        source_workbook.save("temp.xlsx");
+        source_workbook.save("temp" + source.filename());
 
 #ifdef _MSC_VER
         std::ifstream source_stream(source.wstring(), std::ios::binary);

tests/workbook/workbook_test_suite.cpp

@@ -66,6 +66,7 @@ public:
         register_test(test_comparison);
         register_test(test_id_gen);
         register_test(test_load_file);
+        register_test(test_Issue279);
     }
 
     void test_active_sheet()
@@ -460,11 +461,35 @@ public:
         // load with vector
         std::ifstream file_reader3(file.string(), std::ios::binary);
         file_reader3.unsetf(std::ios::skipws);
-        std::vector<uint8_t> data(std::istream_iterator<uint8_t>{file_reader3}, 
+        std::vector<uint8_t> data(std::istream_iterator<uint8_t>{file_reader3},
-                                  std::istream_iterator<uint8_t>());
+            std::istream_iterator<uint8_t>());
         xlnt::workbook wb_load5;
         wb_load5.load(data);
         xlnt_assert_equals(wb_path, wb_load5);
     }
+
+    void test_Issue279()
+    {
+        xlnt::workbook wb(path_helper::test_file("Issue279_workbook_delete_rename.xlsx"));
+        while (wb.sheet_count() > 1)
+        {
+            if (wb[1].title() != "BOM")
+            {
+                wb.remove_sheet(wb[1]);
+            }
+            else
+            {
+                wb.remove_sheet(wb[0]);
+            }
+        }
+        // get sheet bom change title
+        auto ws1 = wb.sheet_by_index(0);
+        ws1.title("checkedBom");
+        // report sheet
+        auto ws2 = wb.create_sheet(1);
+        ws2.title("REPORT");
+        //save a copy file
+        wb.save("temp.xlsx");
+    }
 };
 static workbook_test_suite x;

tests/worksheet/range_test_suite.cpp

@@ -37,10 +37,40 @@ class range_test_suite : public test_suite
 public:
     range_test_suite()
     {
+        register_test(test_construction);
         register_test(test_batch_formatting);
         register_test(test_clear_cells);
     }
 
+    void test_construction()
+    {
+        xlnt::workbook wb;
+        auto ws = wb.active_sheet();
+
+        xlnt::range range_1(ws, xlnt::range_reference("A1:D10"));
+        xlnt_assert_equals(range_1.target_worksheet(), ws);
+        xlnt_assert_equals(1, range_1.front()[0].row()); // NOTE: querying row/column here desperately needs some shortcuts
+        xlnt_assert_equals(xlnt::column_t("D"), range_1.front().back().column());
+        xlnt_assert_equals(10, range_1.back()[0].row());
+        xlnt_assert_equals(xlnt::column_t("D"), range_1.back().back().column());
+        // assert default parameters in ctor
+        xlnt::range range_2(ws, xlnt::range_reference("A1:D10"), xlnt::major_order::row, false);
+        xlnt_assert_equals(range_1, range_2);
+        // assert copy
+        xlnt::range range_3(range_2);
+        xlnt_assert_equals(range_1, range_3);
+
+        // column order
+        xlnt::range range_4(ws, xlnt::range_reference("A1:D10"), xlnt::major_order::column);
+        xlnt_assert_equals(xlnt::column_t("A"), range_4.front()[0].column()); // NOTE: querying row/column here desperately needs some shortcuts
+        xlnt_assert_equals(10, range_4.front().back().row());
+        xlnt_assert_equals(xlnt::column_t("D"), range_4.back()[0].column());
+        xlnt_assert_equals(10, range_4.back().back().row());
+        // assignment
+        range_3 = range_4;
+        xlnt_assert_equals(range_3, range_4);
+    }
+
     void test_batch_formatting()
     {
         xlnt::workbook wb;

tests/worksheet/worksheet_test_suite.cpp

@@ -50,7 +50,6 @@ public:
         register_test(test_remove_named_range_bad);
         register_test(test_cell_alternate_coordinates);
         register_test(test_cell_range_name);
-        register_test(test_hyperlink_value);
         register_test(test_rows);
         register_test(test_no_rows);
         register_test(test_no_cols);
@@ -225,50 +224,6 @@ public:
         xlnt_assert(c_range_coord[0][0] == c_cell);
     }
 
-    void test_hyperlink_value()
-    {
-        xlnt::workbook wb;
-        auto ws = wb.active_sheet();
-        std::string test_link = "http://test.com";
-        xlnt::cell_reference test_cell("A1");
-        ws.cell(test_cell).hyperlink(test_link);
-        // when a hyperlink is added to an empty cell, the display text becomes == to the link
-        xlnt_assert_equals(ws.cell(test_cell).hyperlink().url(), test_link);
-        xlnt_assert_equals(ws.cell(test_cell).value<std::string>(), test_link);
-        // if the display value changes, the hyperlink remains the same
-        std::string test_string = "test";
-        ws.cell(test_cell).value(test_string);
-        xlnt_assert_equals(test_string, ws.cell(test_cell).value<std::string>());
-        xlnt_assert_equals(ws.cell(test_cell).hyperlink().url(), test_link);
-        // changing the link doesn't change the cell value
-        std::string test_link2 = "http://test-123.com";
-        ws.cell(test_cell).hyperlink(test_link2);
-        xlnt_assert_equals(test_string, ws.cell(test_cell).value<std::string>());
-        xlnt_assert_equals(ws.cell(test_cell).hyperlink().url(), test_link2);
-        // and we can edit both value and hyperlink together
-        std::string test_string3 = "123test";
-        std::string test_link3 = "http://123-test.com";
-        ws.cell(test_cell).hyperlink(test_link3, test_string3);
-        xlnt_assert_equals(test_string3, ws.cell(test_cell).value<std::string>());
-        xlnt_assert_equals(ws.cell(test_cell).hyperlink().url(), test_link3);
-        // hyperlinks can also be applied to cells with non-string values
-        int numeric_test_val = 123;
-        std::string test_link4 = "http://test-numeric.com";
-        xlnt::cell_reference numeric_test_cell("B1");
-        ws.cell(numeric_test_cell).value(numeric_test_val);
-        ws.cell(numeric_test_cell).hyperlink(test_link4);
-        xlnt_assert_equals(ws.cell(numeric_test_cell).hyperlink().url(), test_link4);
-        xlnt_assert_equals(ws.cell(numeric_test_cell).value<int>(), numeric_test_val);
-        // and there should be no issues if two cells use the same hyperlink
-        ws.cell(numeric_test_cell).hyperlink(test_link3); // still in use on 'A1'
-        // 'A1'
-        xlnt_assert_equals(test_string3, ws.cell(test_cell).value<std::string>());
-        xlnt_assert_equals(ws.cell(test_cell).hyperlink().url(), test_link3);
-        // 'B1'
-        xlnt_assert_equals(ws.cell(numeric_test_cell).hyperlink().url(), test_link3);
-        xlnt_assert_equals(ws.cell(numeric_test_cell).value<int>(), numeric_test_val);
-    }
-
     void test_rows()
     {
         xlnt::workbook wb;