RFR (M) 8210689: Remove the multi-line old C style for string literals

[JC Beyler]

Hi Chris,

Sounds good to me; here it is:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8210689

I admit I strived to stay consistent and always started a new line for the
multi-line argument even if the string was not too long; it's a question of
style I believe but it felt more readable to me. I'll happily change
whatever anyone prefers.

This has passed the vmTestbase tests I changed but due to the shared
changes, I've launched a full vmTestbase testing now.

Let me know what you think,
Jc

On Fri, Sep 21, 2018 at 10:59 AM Chris Plummer <chris.plummer at oracle.com>
wrote:

> On 9/21/18 10:55 AM, JC Beyler wrote:
>
> Hi Chris,
>
> I hesitated to be honest and then thought that debug_str was better as you
> would clearly see that it is a multi-lilne string and what parameters are
> what. But I'll take your preference (it's relatively the same for me).
> Quick question though:
>
> Do you have a preference between:
>
>             NSK_COMPLAIN6(
>                 "TEST FAILED: %s field \"%s\" has\n"
>                 "\tsignature: \"%s\"\n"
>                 "\tgeneric signature: \"%s\"\n\n"
>                 "\tExpected: \"%s\"\n"
>                 "\t\t\"%s\"\n\n",
>                (instance==0)?"instance":"static",
>                 fld_sig[idx][0],
>                 sign, (gen_sign==NULL)?"NULL":gen_sign,
>                 fld_sig[idx][2], fld_sig[idx][3]);
>
>  or:
>
>             NSK_COMPLAIN6(
>                 "TEST FAILED: %s field \"%s\" has\n\tsignature: \"%s\"\n"
>                 "\tgeneric signature: \"%s\"\n\n\tExpected: \"%s\"\n\t\t\"%s\"\n\n",
>                (instance==0)?"instance":"static",
>                 fld_sig[idx][0],
>                 sign, (gen_sign==NULL)?"NULL":gen_sign,
>                 fld_sig[idx][2], fld_sig[idx][3]);
>
> I think I like the first because you can clearly see what we want to be printed out; but for code vertical
>
> compression, the second is better. What do you think?
>
> I also prefer the first one.
>
> thanks,
>
> Chris
>
> Thanks!
>
> Jc
>
>
> On Fri, Sep 21, 2018 at 10:16 AM Chris Plummer <chris.plummer at oracle.com>
> wrote:
>
>> Hi JC,
>>
>> I didn't realize you intended to move all the strings into a "const
>> char*" first. Seems unnecessary, and I think not as easy to read:
>>
>>  138         const char* debug_str =
>>  139             "TEST FAILED: JVMTI_EVENT_CLASS_LOAD event received
>> for\n"
>>  140             "\t a primitive class/array of primitive types with the
>> signature \"%s\"\n";
>>  141         NSK_COMPLAIN1(debug_str, sig);
>>
>> vs
>>
>>  138         NSK_COMPLAIN1("TEST FAILED: JVMTI_EVENT_CLASS_LOAD event
>> received for\n"
>>  139                       "\t a primitive class/array of primitive types
>> with the signature \"%s\"\n",
>>  140                       sig);
>>
>> or
>>
>>  138         NSK_COMPLAIN1(
>>  139             "TEST FAILED: JVMTI_EVENT_CLASS_LOAD event received
>> for\n"
>>  140             "\t a primitive class/array of primitive types with the
>> signature \"%s\"\n",
>>  141             sig);
>>
>> thanks,
>>
>> Chris
>>
>> On 9/21/18 8:00 AM, JC Beyler wrote:
>>
>> Hi all,
>>
>> Is anyone motivated on a Friday to review this ? :)
>>
>> It should be fairly straightforward :-)
>>
>> Thanks,
>> Jc
>>
>> On Tue, Sep 18, 2018 at 12:07 PM JC Beyler <jcbeyler at google.com> wrote:
>>
>>> Hi all,
>>>
>>> Could I have a review for this webrev:
>>>
>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8210689/webrev.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210689
>>>
>>> Let me know what you think,
>>> Jc
>>>
>>
>>
>> --
>>
>> Thanks,
>> Jc
>>
>>
>>
>
> --
>
> Thanks,
> Jc
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180921/f09fb81e/attachment-0001.html>