Time improvements

[doomlaur]

As promised, this pull request does the following:

1. Moves the wrappers for the C time functions to a common file (they no longer exist twice).
2. Uses localtime_r on Linux and other Unix systems that support it to improve thread-safety.
3. Fixes the unit test for weekdays under MSVC.

Note: the improved wrappers for the C time functions are based on the previous ones, but contain the improvements stated above. However, this also means that the new wrappers - just like the old ones - don't have any handling for failed calls to localtime or gmtime. For example, under MSVC, the C Standard Library of MSVC has the limitation for localtime, gmtime and mktime that it only supports dates between January 1st 1970 00:00:00 UTC and December 31st 3000 23:59:59 UTC. However, Excel supports dates beginning with January 1st, 1900, while XLNT under MSVC would not handle it correctly, without any information to the caller of our safe functions that something failed. We could change that by returning an xlnt::optional in case we detect that something has failed. Currently, the following will happen:

• for localtime_s and gmtime_s (under MSVC), as well as localtime_r and gmtime_r under Unix, a default-initialized std::tm would be returned (that is not useful in most cases, of course)
• for std::localtime and std::gmtime (on other systems than Windows and Unix), we would dereference a nullptr and cause a crash changed to no longer cause a crash and behave like on Windows and Unix - but this still means that the caller has no idea that the call failed

I definitely think that this is not great and should be improved - but I created this pull request to first ask what you @m7913d think about my proposed solution 🙂

[coveralls]

Pull Request Test Coverage Report for Build db3b7966-1e9d-4fbf-ab64-e115ee9942d8

Details

• 122 of 129 (94.57%) changed or added relevant lines in 6 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.06%) to 83.21%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
source/utils/datetime.cpp	51	52	98.08%
source/utils/date.cpp	29	31	93.55%
source/utils/time.cpp	0	4	0.0%

Totals
Change from base Build dc7e29c9-72c4-42b0-a61d-2e9c99f48c75:	0.06%
Covered Lines:	10720
Relevant Lines:	12883

---

💛 - Coveralls

[m7913d]

Concerning error handling: I would be fine with returning a xlnt::optional object by your new *_safe functions.
However, the only function that reasonably may fail is date::weekday (as the others use the current date/time as input), but I think this function doesn't need to use localtime_safe (see below).

[m7913d]

As a minor remark, coverage is decreased due to the is_null conditions not being unit-tested (ensure exception is thrown, resulting string is empty). See coverage report.

[doomlaur]

Good point, thank you! I added unit tests for covering all cases that might fail due to an invalid xlnt::date or xlnt::datetime.

[doomlaur]

And, once again, CI has caught a case where all unit tests passed on MSVC, but failed on Linux. Nice! 😄

[m7913d]

Just some minor remarks. We are almost there.

[m7913d]

Thanks for all the changes!