[ 
https://issues.apache.org/jira/browse/THRIFT-5497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17477465#comment-17477465
 ] 

Richard H. Gumpertz edited comment on THRIFT-5497 at 1/18/22, 1:18 AM:
-----------------------------------------------------------------------

I have three important questions:

(1) What should the *default ::apache::thrift::to_string* format be for 
displaying binary_strings?  Embedding lots of *\x* sequences is ugly and hard 
to read: *"\Xde\Xad\Xbe\Xef"* !!!
I'm very much leaning toward  {*}X"deadbeef"{*}.  One could also consider the 
C++ syntax *"deadbeef"X* _{_}(using syntax like a C++ _user-defined literal 
operator{_} ""{_}X, but that could conflict with an end{_} _user application's 
use of the operator ""_X).  Furthermore, it has a readability problem: one's 
eye tends not to see the X when it is at the end of the string; it it much more 
attention-getting at the beginning.  *_Anyway, PLEASE SUBMIT SUGGESTIONS!*_

(2) Due to unfamiliarity with the general layout of the Thrift include files, I 
am a bit unsure where to put the definition of 
:{*}:apache::thrift::binary_string{*}.   I believe the header file Thrift.h is 
the appropriate place but would very much appreciate confirmation.  *{_}Any 
feedback from the more experienced Thrift community{_}?* 

(3) Is *::apache::thrift::to_string* used just for displaying strings to users 
or is it also used in non-binary protocols for passing data?  If the latter, 
must any reader(s) or other functions be able to parse strings generated by 
to_string?  *_If so, which?_*  

Finally, here is the latest working definition of 
::apache::thrift::binary_string: [^binary_string_2022.01.17_#2.h] .  I believe 
it may finally be fairly stable,


was (Author: JIRAUSER283319):
I have three important questions:

(1) What should the *default ::apache::thrift::to_string* format be for 
displaying binary_strings?  Embedding lots of *\x* sequences is ugly and hard 
to read: *"\Xde\Xad\Xbe\Xef"* !!!
I'm very much leaning toward  {*}X"deadbeef"{*}.  One could also consider the 
C++ syntax *"deadbeef"X* __(using syntax like a C++ _user-defined literal 
operator_ ""{_}X, but that could conflict with an user application's use of the 
operator ""_X).  Furthermore, it has a readability problem: one's eye tends not 
to see the X when it is at the end of the string; it it much more 
attention-getting at the beginning.  *_Anyway, PLEASE SUBMIT SUGGESTIONS!*{_}

(2) Due to unfamiliarity with the general layout of the Thrift include files, I 
am a bit unsure where to put the definition of 
:{*}:apache::thrift::binary_string{*}.   I believe the header file Thrift.h is 
the appropriate place but would very much appreciate confirmation.  *{_}Any 
feedback from the more experienced Thrift community{_}?* 

(3) Is *::apache::thrift::to_string* used just for displaying strings to users 
or is it also used in non-binary protocols for passing data?  If the latter, 
must any reader(s) or other functions be able to parse strings generated by 
to_string?  *_If so, which?_*  

Finally, here is the latest working definition of 
::apache::thrift::binary_string: [^binary_string_2022.01.17_#2.h] .  I believe 
it may finally be fairly stable,

> Change the C++ representation of binary be std::basic_string<unsigned_char>
> ---------------------------------------------------------------------------
>
>                 Key: THRIFT-5497
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5497
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Richard H. Gumpertz
>            Priority: Minor
>         Attachments: binary_string_2022.01.17_#2.h
>
>
> I would like to change the C++ representation of binary from "std::string" to 
> "std::basic_string<unsigned_char>".  This would allow C++ templates to 
> distinguish the Thrift type "string" from "binary".
> If nothing else, ::apached::thrift::to_string could print binary differently 
> than string.  Currently, it can get pretty ugly when one calls printTo on a 
> struct containing a binary field that has data that is not readable!
> There is an issue as to how they should be displayed by default; using 
> \xDE\xAD\xBE\xEF with a \ before every byte seems ugly.  Alternativlely, 
> "DEADBEEF"_x because it could also be used as input if the C++ operator""_x 
> is defined by the user?  Alternatively just <binary> or <27-byte binary> 
> (mirroring <null> for unset fields), hiding the contents?  Perhaps leave 
> to_string(std::basic_string<unsigned char> &value) undefined and force the 
> user to define it if needed?  Thoughts & suggestions???
> Given that such a change could break existing code, it should probably be 
> enabled under a new thrift compiler option to --gen cpp.  I haven't come up 
> with a good name for such an option yet: --gen cpp:binary_as_unsigned_string 
> is explicit but wordy.  Any suggestions???
> I would be willing to do the work necessary to add the relevant code if there 
> are no volunteers who have a better understanding of precisely what code 
> would have to be updated (or at least help me find such code)



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to