Fixes to cell hyper links

-- additional tests to cover both variants of the set function
-- additional tests to cover hyper links on cells with non-string data
-- modified hyper link set function to handle cells with non-string data
-- fixed test behaviour for a cell with no value (value is set to the string form of the link. Behaviour copied from Excel 2010)

source/cell/cell.cpp

@@ -369,12 +369,7 @@ hyperlink cell::hyperlink() const
 
 void cell::hyperlink(const std::string &url)
 {
-    hyperlink(url, url);
+    if (url.empty() || std::find(url.begin(), url.end(), ':') == url.end())
-}
-
-void cell::hyperlink(const std::string &url, const std::string &display)
-{
-    if (url.empty() || std::find(url.begin(), url.end(), ':') == url.end() || display.empty())
     {
         throw invalid_parameter();
     }
@@ -386,18 +381,14 @@ void cell::hyperlink(const std::string &url, const std::string &display)
     d_->hyperlink_ = detail::hyperlink_impl();
 
     // check for existing relationships
-    for (const auto &rel : manifest.relationships(ws.path(), relationship_type::hyperlink))
+    auto relationships = manifest.relationships(ws.path(), relationship_type::hyperlink);
-    {
+    auto relation = std::find_if(relationships.cbegin(), relationships.cend(),
-        if (rel.target().path().string() == url)
+        [&url](xlnt::relationship rel) { return rel.target().path().string() == url; });
-        {
+    if (relation != relationships.end()) {
-            d_->hyperlink_.get().relationship = rel;
+        d_->hyperlink_.get().relationship = *relation;
-            existing = true;
-            break;
     }
-    }
+    else
-
+    { // register a new relationship
-    // register a new relationship
-    if (!existing) {
         auto rel_id = manifest.register_relationship(
             uri(ws.path().string()),
             relationship_type::hyperlink,
@@ -407,7 +398,23 @@ void cell::hyperlink(const std::string &url, const std::string &display)
         d_->hyperlink_.get().relationship = manifest.relationship(ws.path(), rel_id);
     }   
     
+    if (!has_value()) // hyperlink on an empty cell sets the value to the hyperlink string
+    {
+        value(url);
+    }
+}
+
+void cell::hyperlink(const std::string &url, 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;
 }
 

tests/worksheet/worksheet_test_suite.hpp

@@ -225,12 +225,44 @@ public:
     {
         xlnt::workbook wb;
         auto ws = wb.active_sheet();
-        ws.cell("A1").hyperlink("http://test.com");
+        std::string test_link = "http://test.com";
-        xlnt_assert_equals(ws.cell("A1").hyperlink().url(), "http://test.com");
+        xlnt::cell_reference test_cell("A1");
-        xlnt_assert_equals(ws.cell("A1").value<std::string>(), "");
+        ws.cell(test_cell).hyperlink(test_link);
-        ws.cell("A1").value("test");
+        // when a hyperlink is added to an empty cell, the display text becomes == to the link
-        xlnt_assert_equals("test", ws.cell("A1").value<std::string>());
+        xlnt_assert_equals(ws.cell(test_cell).hyperlink().url(), test_link);
-        xlnt_assert_equals(ws.cell("A1").hyperlink().url(), "http://test.com");
+        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()