Added support for pre-c++17 experimental string_view #607
Conversation
|
FIxed one wrong #if FMT_HAS_(EXPERIMENTAL_)STRING_VIEW added in commit before |
vitaut
left a comment
There was a problem hiding this comment.
Thanks for the PR! Mostly looks good, just a few minor comments inline.
| # define FMT_HAS_STRING_VIEW 1 | ||
| # define FMT_HAS_EXPERIMENTAL_STRING_VIEW 0 | ||
| #else | ||
| # define FMT_HAS_STRING_VIEW 0 |
There was a problem hiding this comment.
I suggest keeping this and removing 2 identical defines below since they don't depend on whether experimental string_view exists.
There was a problem hiding this comment.
Can't get your point, can you paste here how you want the final code to look like?
There was a problem hiding this comment.
#else
# define FMT_HAS_STRING_VIEW 0
# if (FMT_HAS_INCLUDE(<experimental/string_view>) && __cplusplus > 201402L)
# include <experimental/string_view>
# define FMT_HAS_EXPERIMENTAL_STRING_VIEW 1
# else
# define FMT_HAS_EXPERIMENTAL_STRING_VIEW 0
# endif|
|
||
| #if FMT_HAS_EXPERIMENTAL_STRING_VIEW | ||
| /** | ||
| \rst |
There was a problem hiding this comment.
nit: Please indent the rst block for consistency with the rest of the code.
| target_include_directories(gmock PUBLIC .) | ||
|
|
||
| if (CYGWIN) | ||
| target_compile_definitions(gmock PUBLIC _POSIX_C_SOURCE=200809) |
There was a problem hiding this comment.
This is because google test fails to run on Cygwin because they doesn't define _POSIX_C_SOURCE and posix api´s (fileno, strdup, fdopen, and others).
See reference https://stackoverflow.com/questions/18784112/googletest-1-6-with-cygwin-1-7-compile-error-fileno-was-not-declared-in-this
There was a problem hiding this comment.
Could you add a comment explaining this?
Comments about CYGWIN tests for building
|
Take a look at this last commit, if it fits your requests. |
|
Merged, thanks! |
Also, added _POSIX_C_SOURCE to make library compile and pass on all tests on Cygwin environment.