-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feature request] std::string support for json library #72
Comments
I would love to see this as a feature as well. Also huge thanks to petke for showing how to get a string variable into a json container. While you may not be able to update this as a feature, can anyone update the documentation with this in json, it would be a huge help to a lot of people including myself. EDIT: For help to anyone else trying to convert from wstring to string without using the conversion utilities.
|
I definitely agree that the wstring/string/string_t issues cause a ton of headache when using the library. Let me lay out the current issues I see that need to be solve here.
There are two obvious options that I'll throw out to start with:
What do you think? |
in my opinion the cleanest solution would be for core json types (web::json::value) etc to be templated on the string type so it can use either std::string and std::wstring. That way the user gets to choose which to use, but the implementation logic for both stays (much) the same and does not have to be forked. Kindof how in the c++ standard library itself, the std::basic_string is templated on the character type so that it can use either char or wchar. Something similar should be possible here. |
+1 |
+1 |
You are all free to submit patches to correct this! Sent from my Nexus 5.
|
Hi Graham, are you working at Microsoft and cpprest? I suspect this is such a fundamental design change of the library, that one would like some feedback from the official maintainers first. Why did they decide to support only one or the other? Maybe there is some good reason for the current design. Do they have any interest in such a feature, or are they against it on principle? |
No, I don't work for MS. ras0219-msft did give feedback: Either lightweight wrapper or full-on conversion for version 3.0.0. I did write a wrapper to do exactly what you mentioned in the the original post and what @ras0219-msft mentions in his (1) in a previous job because manually converting to and fro was a nuisance and a code bloat. Because of the wrapper we were able to pretty much ignore the problem. Question is how many other teams have written similar objects to hide the pain? A light weight wrapper would get my vote (I don't use cpprestsdk in my new job) but I would like to see a full conversion at some point in the future (you never know what I might have need of it again). |
Thanks graham. I somehow missed that ras...msft works for ms. Maybe ras and friends will fix it for all at some point. Thanks |
Is anyone working on this right now? In our code base we use utf8 everywhere (yes, even on Windows) and it is a big pain to have to do string conversions everywhere. |
I found that this could be solved by adding a build option (you would need to build cpprestsdk yourself) and updating some of the #ifdef _WIN32 to #if defined(_UTF16_STRINGS). I will be creating a PR to add this support. |
+1 Jason. Nice! |
I have not fully validated the changes (been building just the json support portions), but I pushed changes at https://github.com/jason-ha/cpprestsdk.git fork in branch win32_narrow_strings if anyone want to give it a whirl. |
I just wanna use |
@NickYang1988 No, Windows made moved to reduce overall string complexity by supporting UNICODE with 16 characters by default. I have verified more of my attempt to support this for json parsing. As described well by @ras0219-msft there are complications - particularly cpprestsdk having a common utility::string_t across all code. Thus I am working on an additional change to better componentize the suite. With this change cpprest_json and cpprest_pplx static libraries can be produced using new CPPREST_ISOLATE_COMPONENTS build option. |
Thanks for your reply, I know Windows use And I read the repo with |
So you are getting that exception during the call to to_string_t. The exception is thrown from count_utf8_to_utf16. I think you need to validate the source of your UTF8 string. The exception is being thrown because an invalid encoding has been detected. UTF-8 use high bit (1xxxxxxx) to indicated extended character. But it also requires that bit 6 (8 bits 0-7; I call this out because cpprest uses 1-8 bit position notation) be 1, ie: 11xxxxxx, but you must have 10xxxxxx. |
branch win32_narrow_strings has been rebased to 2.10.14 and cleanups have been made to enable a clean (but not full) cpprest package. |
At this point we can't build the listener and client on Windows with UTF8, correct? |
In my experience most 3rd party libraries use std::string and not std::wstring in their interface. Therefore applications become tied to std::string. Because the ppl/json library uses std::wstring (on windows) this makes integrating the ppl/json library into an applications cumbersome and inefficient. One has to do lots of unnecessary wstring-to-string and string-to-wstring conversions. Could you add support for std::string, at least for the json library?
Minimal example:
The text was updated successfully, but these errors were encountered: