[spdlog] support wchar when target is Windows#18041
Merged
BillyONeal merged 14 commits intomicrosoft:masterfrom Jul 15, 2021
luncliff:port/spdlog
Merged
[spdlog] support wchar when target is Windows#18041BillyONeal merged 14 commits intomicrosoft:masterfrom luncliff:port/spdlog
BillyONeal merged 14 commits intomicrosoft:masterfrom
luncliff:port/spdlog
Conversation
* filename will be `std::string` * functions `wstr_to_utf8buf`, `utf8_to_wstrbuf` will be added
luncliff
changed the title
JonLiu1993
approved these changes
May 21, 2021
JackBoosY
added
the
category:port-bug
JackBoosY
suggested changes
May 21, 2021
* new feature: `wchar`, `wchar-filenames` * update git-tree SHA
JackBoosY
approved these changes
May 24, 2021
JackBoosY
added
info:needs-maintainer-attention
strega-nil-ms
suggested changes
Jun 2, 2021
Contributor
strega-nil-ms
left a comment
strega-nil-ms
left a comment
There was a problem hiding this comment.
wchar is a great feature. However, wchar-filenames is not a feature. Features must be additive, and must not change existing APIs.
The correct way forward is via a triplet option. I can't find a good example, but basically, you'd set it up so that in a triplet you can write:
if (PORT STREQUAL "spdlog")
set(SPDLOG_WCHAR_FILENAMES ON)
endif()You can see how you might do this via the suggestions
strega-nil-ms
added
requires:author-response
and removed
info:needs-maintainer-attention
Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
Co-authored-by: nicole mazzuca <83086508+strega-nil-ms@users.noreply.github.com>
JackBoosY
suggested changes
Jun 7, 2021
JackBoosY
approved these changes
Jun 7, 2021
Contributor
|
@strega-nil-ms Ping for review again. |
Contributor
|
Possible to have a bump on this one ? |
Contributor
|
@strega-nil-ms Please help review this pr. |
JackBoosY
added
category:port-feature
PhoebeHui
reviewed
Jun 21, 2021
Contributor
ras0219-msft
left a comment
ras0219-msft
left a comment
There was a problem hiding this comment.
Thanks for the PR and sorry for the delay! Just a few tiny tweaks needed before merging :)
JonLiu1993
added
requires:author-response
and removed
requires:vcpkg-team-review
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
PhoebeHui
added
info:needs-maintainer-attention
strega-nil-ms
approved these changes
Jul 2, 2021
Member
|
Thanks for your contribution and sorry for the delay! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does your PR fix?
Possible fix of #16926? I think the error message can be incorrect because spdlog uses macro guards to show/hide overloaded functions.
SPDLOG_WCHAR_SUPPORT
Adds 2 functions into DLL (
spdlog::spdlogIMPORTED target)wstr_to_utf8bufutf8_to_wstrbufBecame a port feature. It can be activated with
spdlog[wchar].The library double checks this macro with
_WIN32, so the other platforms won't need this.SPDLOG_WCHAR_FILENAMES
This option makes interfaces to use
std::wstringinstead ofstd::string.ON for this option will break windows users' builds.
Became a port feature. It can be activated withspdlog[wchar-filenames].Which triplets are supported/not supported? Have you updated the CI baseline?
Windows(
windows,uwp) triplets will be affected.Does your PR follow the maintainer guide?
Checked.