support unicode in paths

[xfc1939]

Fixes #984

[codecov-commenter]

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (931323d) 64.14% compared to head (0932ea5) 64.32%. Report is 8 commits behind head on master.

Files	Patch %	Lines
src/logging.cc	80.00%	3 Missing and 4 partials :warning:
src/utilities.cc	60.00%	1 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #998      +/-   ##
==========================================
+ Coverage   64.14%   64.32%   +0.18%     
==========================================
  Files          17       17              
  Lines        3327     3330       +3     
  Branches     1125     1126       +1     
==========================================
+ Hits         2134     2142       +8     
+ Misses        766      760       -6     
- Partials      427      428       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[sergiud]

Thanks for the PR. I need some time to look through the changes.

In the meantime, could you please rebase onto current head and resolve the conflicts?

[xfc1939]

Now I have modified the DefaultDir() function by used template. Could you give me some suggestions.

template<class CharT>
static const std::basic_string<typename std::enable_if_t<std::is_same_v<CharT, wchar_t>, CharT>> DefaultLogDir() {
        std::wstring env = getenvw((wchar_t*)TEXT("GOOGLE_LOG_DIR"));
        if (!env.empty()) {
            return env;
        }
        env = getenvw((wchar_t*)TEXT("TEST_TMPDIR"));
        return env;
}

template<class CharT>
static const std::basic_string<typename  std::enable_if_t<std::is_same_v<CharT, char>, CharT>> DefaultLogDir() 
{
    const char* env;
    env = getenv("GOOGLE_LOG_DIR");
    if (env != nullptr && env[0] != '\0') {
        return env;
    }
    env = getenv("TEST_TMPDIR");
    if (env != nullptr && env[0] != '\0') {
        return env;
    }
    return "";
}

template<class CharT>
static const std::basic_string<typename  std::enable_if_t<!std::is_same_v<CharT, char> && !std::is_same_v<CharT, wchar_t>, CharT>> DefaultLogDir()
{
    return {};
}

[sergiud]

In the code above, DefaultDir sill uses duplicated logic while the only difference is the use of getenv for ANSI and Unicode strings. It is therefore more meaningful to provide a unified overload for the corresponding functions.

Something like this should do the job:

namespace details {
using std::getenv; // char variant
const wchar_t* getenv(const wchar_t* name) {
    return _wgetenv(name);
}
} // namespace internal

template<class Ch, std::size_t N>
std::basic_string<Ch> DefaultLogDir(const std::array<const Ch*, N>& names) {
    for (const Ch* name : names) {
        if ((const Ch* var = details::getenv(name)) != nullptr) {
            return var;
        }
    }
    return {};
}

Now you only need to specialize the literals:

template<class Ch>
struct LogDirEnvVar;

template<>
struct LogDirEnvVar<char> {
    constexpr static std::array<const char*, 2> names() noexcept {
        return {"GOOGLE_LOG_DIR", "TEST_TMPDIR"};
    }
};

template<>
struct LogDirEnvVar<wchar_t> {
    constexpr static std::array<const wchar_t*, 2> names() noexcept {
        return {L"GOOGLE_LOG_DIR", L"TEST_TMPDIR"};
    }
};

Eventually, you can define

template<class Ch>
decltype(auto) DefaultLogDir() {
    return DefaultLogDir(LogDirEnvVar<Ch>::names());
}

and use it as follows:

auto dir1 = DefaultLogDir<char>();
auto dir2 = DefaultLogDir<wchar_t>();

However, I'm not sure this is really necessary here. Can't you just always use the wide string variant here?

[xfc1939]

In the code above, DefaultDir sill uses duplicated logic while the only difference is the use of getenv for ANSI and Unicode strings. It is therefore more meaningful to provide a unified overload for the corresponding functions.

Something like this should do the job:

namespace details {
using std::getenv; // char variant
const wchar_t* getenv(const wchar_t* name) {
    return _wgetenv(name);
}
} // namespace internal

template<class Ch, std::size_t N>
std::basic_string<Ch> DefaultLogDir(const std::array<const Ch*, N>& names) {
    for (const Ch* name : names) {
        if ((const Ch* var = details::getenv(name)) != nullptr) {
            return var;
        }
    }
    return {};
}

Now you only need to specialize the literals:

template<class Ch>
struct LogDirEnvVar;

template<>
struct LogDirEnvVar<char> {
    constexpr static std::array<const char*, 2> names() noexcept {
        return {"GOOGLE_LOG_DIR", "TEST_TMPDIR"};
    }
};

template<>
struct LogDirEnvVar<wchar_t> {
    constexpr static std::array<const wchar_t*, 2> names() noexcept {
        return {L"GOOGLE_LOG_DIR", L"TEST_TMPDIR"};
    }
};

Eventually, you can define

template<class Ch>
decltype(auto) DefaultLogDir() {
    return DefaultLogDir(LogDirEnvVar<Ch>::names());
}

and use it as follows:

auto dir1 = DefaultLogDir<char>();
auto dir2 = DefaultLogDir<wchar_t>();

However, I'm not sure this is really necessary here. Can't you just always use the wide string variant here?

Thanks for your suggestions; it's a very nice thing for me. Regarding your question, I don't have much experience with platforms other than Windows, but I will try to do it.

[sergiud]

@xfc1939 Please do not close the PRs to create a new one. Simply update your branch and push the changes. Thank you.

[xfc1939]

@xfc1939 Please do not close the PRs to create a new one. Simply update your branch and push the changes. Thank you.

all right, I have updated my branch and sumited here.