add option to directly support narrow strings on win32 #1199
Conversation
independent of UNICODE vs MBCS setting Set CPPREST_FORCE_NARROW_STRINGS=ON to enable. Simplistic cleanup throughout project is not possible as some Windows APIs require utf16 such as HTTPSPolicyCallbackData in http_client_winhttp.cpp. HTTP related code is not completely updated so a full package may not be produced, though settings exist to have a clean build. Use: -DCPPREST_FORCE_NARROW_STRING=ON -DCPPREST_EXCLUDE_WEBSOCKETS=ON -DCPPREST_HTTP_LISTENER_IMPL=none -DBUILD_SAMPLES=OFF
…omponents like just json support extractions from asyncrt_utils.h: base64_utils.h (to/form base64) memory_utils.h (details::make_unique) string_utils.cpp (string utils) asyncrt_utils retains error, date-time, and nounce utils
|
Look like legitimate build/test failures that need to be fixed. |
|
Unfortunately I don't think it is appropriate to merge this work with this still disabled, as merging this to master is making a statement that it's an officially supported part of the product. However, I acknowledge trying to make changes of this scale is difficult to do as a complete fully formed change if it were to be squashed as a single commit, and doing it as separate reviews certainly makes it easier to review. Are you OK with retargeting this change to a semi-supported branch for this, which we will merge with a non-squash commit later? I reviewed this and agree with all the changes herein (other than the above concern). |
|
I wouldn't say it is disabled as it is an option for Win32. Most Win32 users will not want to enable it.
I do not think squashing is good way to go for this PR. Merge is good as (1) isolated component branch is derived from this and (2) I made targeted commits for preserving history in split files. |
|
I prepped PR #1230 if option 4 is desired. It is exactly this branch plus one change to remove CMakeLists.txt related changes and the basic_types.h use of the option. |
A change like that, as well as the 'isolated components' change, is moving the project in a direction where the json component would be usable on its own, without any HTTP support. For us that is an explicit non-goal. cpprestsdk is an HTTP client, and a not really being maintained much one at that. Extracting the json components into their own project/repo which doesn't build any HTTP components sounds like it might be an interesting project, but isn't a scenario we are interested in supporting at this time. Let me ask bosses et al. if there's a path forward for us here. |
|
FWIW I have now worked on several projects that reference pplx portion of cpprestsdk without using HTTP client portion. The JSON parsing is also on Microsoft's approved list of deserializers. So maybe not for your team directly, but there is value to the parts of cpprestsdk independently. If the maintenance is low, then perhaps it makes more sense to split out the most used/valuable parts. |
|
Right, this is an instance where what's probably the right technical decision might be counter to the current owner's strategic goals. (Namely, this project is no part of the team that currently owns it's charter) That might change.... soon which is why I'm asking around |
|
@BillyONeal been a while, but this json library is still our best option. Any changes such that we might be able to get this merged? |
|
Didn't mean to request review for this one. Meant for the alternate that just handles better ifdef'ing. Per previous discussion this form would be for the semi-supported branch. |
partial support for feature request #72
add option to directly support narrow strings on win32
independent of UNICODE vs MBCS setting
Set CPPREST_FORCE_NARROW_STRINGS=ON to enable.
Simplistic cleanup throughout project is not possible as some Windows APIs require utf16 such as HTTPSPolicyCallbackData in http_client_winhttp.cpp.
HTTP related code is not completely updated so a full package may not be produced, though settings exist to have a clean build. Use:
-DCPPREST_FORCE_NARROW_STRING=ON -DCPPREST_EXCLUDE_WEBSOCKETS=ON -DCPPREST_HTTP_LISTENER_IMPL=none -DBUILD_SAMPLES=OFF